-
Notifications
You must be signed in to change notification settings - Fork 93
Migrate to OCI SDK #148
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Migrate to OCI SDK #148
Conversation
@@ -25,16 +25,16 @@ import ( | |||
"github.com/golang/glog" | |||
|
|||
utilruntime "k8s.io/apimachinery/pkg/util/runtime" | |||
"k8s.io/apimachinery/pkg/util/wait" | |||
"k8s.io/client-go/informers" | |||
wait "k8s.io/apimachinery/pkg/util/wait" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the package is already called wait
so the first part is redundant.
pkg/oci/client/compute.go
Outdated
// GetInstance gets information about the specified instance. | ||
GetInstance(ctx context.Context, id string) (*core.Instance, error) | ||
// GetInstanceByDisplayName gets information about the named instance. | ||
GetInstanceByDisplayName(ctx context.Context, displayName string) (*core.Instance, error) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any reason to ever use this over GetInstanceByNodeName?
pkg/oci/client/compute.go
Outdated
func IsInstanceInTerminalState(instance *core.Instance) bool { | ||
return instance.LifecycleState == core.InstanceLifecycleStateTerminated || | ||
instance.LifecycleState == core.InstanceLifecycleStateTerminating || | ||
instance.LifecycleState == "UNKNOWN" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The client removed this enum I think so i don't think we need it anymore.
} | ||
|
||
func (c *client) CreateCertificate(ctx context.Context, lbID, certificate, key string) (string, error) { | ||
// TODO(apryde): We currently don't have a mechanism for supplying |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a ticket tracking this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pkg/oci/client/load_balancer.go
Outdated
wr = twr | ||
return true, nil | ||
case loadbalancer.WorkRequestLifecycleStateFailed: | ||
return false, fmt.Errorf("WorkRequest %q failed: %s", id, twr.Message) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
errors.WithStack?
pkg/oci/instances.go
Outdated
if err != nil { | ||
return nil, err | ||
return []api.NodeAddress{}, errors.Wrap(err, "GetInstanceByNodeName") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
any reason you aren't returning nil anymore?
pkg/oci/load_balancer_spec.go
Outdated
} | ||
} | ||
certs[s.SSLConfig.Name] = loadbalancer.CertificateDetails{ | ||
CertificateName: common.String(s.SSLConfig.Name), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: you don't need to use these common methods. Just use &
…m task/OKE-14472 to internal * commit '70a07d7273ef81d60464bd8bb18922e39b65a091': removed TCP health check for SSL enabled traffic
Migrates the project to use the official OCI Go SDK.
CHANGELOG
Test status
Closes: #136