Moving to use operator v1 api #125
Conversation
| func (c *consoleOperator) Sync(obj metav1.Object) error { | ||
| startTime := time.Now() | ||
| logrus.Infof("started syncing operator %q (%v)", obj.GetName(), startTime) | ||
| defer logrus.Infof("finished syncing operator %q (%v) \n\n", obj.GetName(), time.Since(startTime)) | ||
|
|
||
| operatorConfig := obj.(*consolev1alpha1.Console) | ||
| operatorConfig := obj.(*consolev1.Console) |
There was a problem hiding this comment.
Pretty sure if you swap Sync() func here for:
func (c *consoleOperator) Sync(obj metav1.Object) error {
startTime := time.Now()
logrus.Infof("started syncing operator %q (%v)", obj.GetName(), startTime)
defer logrus.Infof("finished syncing operator %q (%v) \n\n", obj.GetName(), time.Since(startTime))
operatorConfig := obj.(*consolev1.Console)
if err := c.handleSync(operatorConfig); err != nil {
return err
}
return nil
}
func (c *consoleOperator) handleSync(config *consolev1.Console) error {
switch config.Spec.ManagementState {
case operatorv1.Managed:
logrus.Println("console is in a managed state.")
// handled below
case operatorv1.Unmanaged:
logrus.Println("console is in an unmanaged state.")
return nil
// take a look @ https://github.com/openshift/service-serving-cert-signer/blob/master/pkg/operator/operator.go#L86
case operatorv1.Removed:
logrus.Println("console has been removed.")
return c.deleteAllResources(config)
default:
// TODO should update status
return fmt.Errorf("unknown state: %v", config.Spec.ManagementState)
}
// do we need the if(configChanged) update bits?
outConfig, configChanged, err := sync_v400(c, config)
if err != nil {
return err
}
if configChanged {
// TODO: this should do better apply logic or similar, maybe use SetStatusFromAvailability
_, err = c.operatorClient.Update(outConfig)
if err != nil {
return err
}
}
return nil
}It will eliminate the fields that we no longer care about.
There was a problem hiding this comment.
See here, a previous pass at moving to the v1, before things changed: https://github.com/openshift/console-operator/pull/103/files#diff-69ecbb024dab972c8729678abdf9c0faR132
There was a problem hiding this comment.
It drops the second switch statement entirely. We dont need it yet.
|
/test all |
|
/test e2e-aws-operator |
|
/retest |
1 similar comment
|
/retest |
pkg/console/operator/operator.go
Outdated
| logrus.Println("console is in an unmanaged state.") | ||
| return nil | ||
| // take a look @ https://github.com/openshift/service-serving-cert-signer/blob/master/pkg/operator/operator.go#L86 | ||
| case operatorsv1alpha1.Removed: | ||
| // take a look @ https://github.com/openshift/service-serving-cert-signer/blob/master/pkg/operator/operator.go#L86 |
There was a problem hiding this comment.
We can probably cut some of these old comments.
|
/test all |
1 similar comment
|
/test all |
|
/retest |
2 similar comments
|
/retest |
|
/retest |
|
/test all |
1 similar comment
|
/test all |
|
Fun fun... |
|
/test all |
|
/retest? |
openshift-ci-robot
added
the
size/XL
|
/retest |
2 similar comments
|
/retest |
|
/retest |
|
/test e2e-aws-operator |
|
/retest |
|
500 errors |
|
/test e2e-aws-operator |
|
/test e2e-aws-operator |
|
/retest |
|
I was hopeful we could sneak one by CI 😅 |
|
/lgtm |
openshift-ci-robot
added
lgtm
|
/test e2e-aws-operator |
|
/retest |
|
looks like this needs to be rebased :/ |
|
boo. |
|
@spadgett rebased and pushed 👍 |
| @@ -63,12 +64,20 @@ func TestStub(t *testing.T) { | |||
| { | |||
| name: "Test stubbing secret", | |||
| want: &corev1.Secret{ | |||
| <<<<<<< HEAD | |||
There was a problem hiding this comment.
Wups. Rebase stuff.....
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: benjaminapetersen, jhadvig The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/retest |
|
/test e2e-aws-operator |
1 similar comment
|
/test e2e-aws-operator |
Migrating from
v1alpha1towardsv1for console-operatorWasn't really sure what to do with
v1alpha1fields that we have been using in the codebase. Adding comments to each occurrence, to discuss if that particular part is necessary.@benjaminapetersen PTAL
@enj fyi