-
Notifications
You must be signed in to change notification settings - Fork 126
OCPBUGS-3440: Restart authentication operator if console capability is enabled #589
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -42,13 +42,19 @@ func TestObserveConsoleURL(t *testing.T) { | |
| clusterVersion: &configv1.ClusterVersionStatus{Capabilities: configv1.ClusterVersionCapabilitiesStatus{EnabledCapabilities: []configv1.ClusterVersionCapability{}}}, | ||
| existingConfig: noConfig, | ||
| expectedConfig: noConfig, | ||
| expectedErrs: []string{ | ||
| "console.config.openshift.io \"cluster\" not found", | ||
| }, | ||
| }, | ||
| { | ||
| // even if the cap is disabled, the observer (if invoked) will take the expected action assuming it | ||
| // can find the console config. | ||
| // the observer should not be invoked if the console cap is disabled. | ||
| name: "ConsoleConfigConsoleCapabilityDisabled", | ||
| consoleConfig: &configv1.ConsoleStatus{ConsoleURL: "https://teh.console.my"}, | ||
| clusterVersion: &configv1.ClusterVersionStatus{Capabilities: configv1.ClusterVersionCapabilitiesStatus{EnabledCapabilities: []configv1.ClusterVersionCapability{}}}, | ||
| existingConfig: configWithConsoleURL(""), | ||
| expectedConfig: configWithConsoleURL(""), | ||
| existingConfig: existingConfig, | ||
| expectedConfig: existingConfig, | ||
| }, | ||
| { | ||
| name: "SameConfig", | ||
|
|
@@ -120,7 +126,7 @@ func TestObserveConsoleURL(t *testing.T) { | |
| } | ||
|
|
||
| for i := range errs { | ||
| if strings.Contains(tt.expectedErrs[i], errs[i].Error()) { | ||
| if !strings.Contains(errs[i].Error(), tt.expectedErrs[i]) { | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this logic was wrong in two ways. as the tests are written, the expected error string is a subset of the actual error string. and of course the test should fail if an error occurs that was not in the expected list, not fail when an expected error occurs. |
||
| t.Errorf("ObserveConsoleURL() errs = %v, want %v", errs, tt.expectedErrs) | ||
| } | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,87 @@ | ||
| package termination | ||
|
|
||
| import ( | ||
| "context" | ||
| "os" | ||
| "time" | ||
|
|
||
| "k8s.io/apimachinery/pkg/util/wait" | ||
| "k8s.io/klog/v2" | ||
|
|
||
| configv1 "github.com/openshift/api/config/v1" | ||
| configinformers "github.com/openshift/client-go/config/informers/externalversions" | ||
| configlistersv1 "github.com/openshift/client-go/config/listers/config/v1" | ||
| "github.com/openshift/library-go/pkg/controller/factory" | ||
| "github.com/openshift/library-go/pkg/operator/events" | ||
| ) | ||
|
|
||
| // terminationController forces a restart of the auth operator when the console capability goes from disabled to enabled. | ||
| // This is necessary to get the console observer registered (which happens during operator startup) so it can manage | ||
| // the console publicAssetUrl, when the console is present. | ||
| // This controller only runs when the operator comes up on a cluster where console is disabled. If the console | ||
| // is enabled when the operator comes up, this controller will not be run. | ||
| type terminationController struct { | ||
| clusterVersionLister configlistersv1.ClusterVersionLister | ||
| recorder events.Recorder | ||
| } | ||
|
|
||
| func NewTerminationController(configInformer configinformers.SharedInformerFactory, recorder events.Recorder) factory.Controller { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. godoc please |
||
| c := &terminationController{ | ||
| clusterVersionLister: configInformer.Config().V1().ClusterVersions().Lister(), | ||
| recorder: recorder, | ||
| } | ||
| return factory.New().WithInformers( | ||
| configInformer.Config().V1().ClusterVersions().Informer(), | ||
| ).ResyncEvery(wait.Jitter(time.Minute, 1.0)).WithSync(c.sync).ToController("TerminationController", recorder.WithComponentSuffix("termination-controller")) | ||
| } | ||
|
|
||
| func (c *terminationController) sync(ctx context.Context, syncCtx factory.SyncContext) error { | ||
|
|
||
| // check the ClusterVersion object to see if the console is currently enabled. | ||
| // Since this controller only runs when the console capability is disabled at operator startup time | ||
| // it is safe to conclude that if it sees the console enabled, it must restart the operator. | ||
| enabled, err := isConsoleCapabilityEnabled(c.clusterVersionLister, c.recorder) | ||
| if err != nil { | ||
| klog.Errorf("Error checking if console capability is enabled: %v", err) | ||
| return err | ||
| } | ||
| if !enabled { | ||
| return nil | ||
| } | ||
| err = triggerRestart() | ||
| if err != nil { | ||
| klog.Errorf("Error triggering restart: %v", err) | ||
| } | ||
| return err | ||
| } | ||
|
|
||
| func isConsoleCapabilityEnabled(clusterVersions configlistersv1.ClusterVersionLister, recorder events.Recorder) (bool, error) { | ||
| clusterVersionConfig, err := clusterVersions.Get("version") | ||
| if err != nil { | ||
| return false, err | ||
| } | ||
|
|
||
| for _, capability := range clusterVersionConfig.Status.Capabilities.EnabledCapabilities { | ||
| if capability == configv1.ClusterVersionCapabilityConsole { | ||
| recorder.Eventf("TerminationController", "Console capability enabled, restarting cluster authentication operator") | ||
| klog.Infof("Console capability enabled, restarting cluster authentication operator") | ||
| return true, nil | ||
| } | ||
| } | ||
| return false, nil | ||
| } | ||
|
|
||
| func triggerRestart() error { | ||
| // this file is an argument to --terminate-on-files in the operator deployment command, so when it is | ||
| // created or updated, the operator will terminate and be restarted. | ||
| f, err := os.OpenFile("/tmp/terminate", os.O_APPEND|os.O_WRONLY|os.O_CREATE, 0600) | ||
| if err != nil { | ||
| klog.Errorf("Failed to restart self due to: %v\n", err) | ||
| return err | ||
| } | ||
| defer f.Close() | ||
| if _, err = f.WriteString(time.Now().String()); err != nil { | ||
| klog.Errorf("Failed to restart self due to: %v\n", err) | ||
| } | ||
| return err | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.