[release-4.12] OCPBUGS-4040: Restart authentication operator if console capability is enabled

manifests/07_deployment.yaml

@@ -35,7 +35,7 @@ spec:
               echo "Copying system trust bundle"
               cp -f /var/run/configmaps/trusted-ca-bundle/ca-bundle.crt /etc/pki/ca-trust/extracted/pem/tls-ca-bundle.pem
           fi
-          exec authentication-operator operator --config=/var/run/configmaps/config/operator-config.yaml --v=2 --terminate-on-files=/var/run/configmaps/trusted-ca-bundle/ca-bundle.crt
+          exec authentication-operator operator --config=/var/run/configmaps/config/operator-config.yaml --v=2 --terminate-on-files=/var/run/configmaps/trusted-ca-bundle/ca-bundle.crt --terminate-on-files=/tmp/terminate
         resources:
           requests:
             memory: 200Mi

pkg/controllers/configobservation/configobservercontroller/observe_config_controller.go

@@ -68,7 +68,6 @@ func NewConfigObserver(
 	for _, o := range []configobserver.ObserveConfigFunc{
 		apiserver.ObserveAdditionalCORSAllowedOrigins,
 		apiserver.ObserveTLSSecurityProfile,
-		console.ObserveConsoleURL,
 		infrastructure.ObserveAPIServerURL,
 		oauth.ObserveIdentityProviders,
 		oauth.ObserveTemplates,
@@ -94,11 +93,12 @@ func NewConfigObserver(
 		PreRunCachesSynced:   preRunCacheSynced,
 	}
 
-	// Check if the Console capability is enabled on the cluster and sync and add its informer and lister.
+	// Check if the Console capability is enabled on the cluster and sync and add its informer, lister, and config observer
 	if enabledClusterCapabilities.Has("Console") {
 		listers.PreRunCachesSynced = append(listers.PreRunCachesSynced, configInformer.Config().V1().Consoles().Informer().HasSynced)
 		informers = append(informers, configInformer.Config().V1().Consoles().Informer())
 		listers.ConsoleLister = configInformer.Config().V1().Consoles().Lister()
+		oauthServerObservers = append(oauthServerObservers, configobserver.WithPrefix(console.ObserveConsoleURL, configobservation.OAuthServerConfigPrefix))
 	}
 
 	return configobserver.NewNestedConfigObserver(

pkg/controllers/configobservation/console/observe_consoleurl.go

@@ -9,8 +9,6 @@ import (
 	"github.com/openshift/library-go/pkg/operator/configobserver"
 	"github.com/openshift/library-go/pkg/operator/events"
 
-	configv1 "github.com/openshift/api/config/v1"
-
 	"github.com/openshift/cluster-authentication-operator/pkg/controllers/configobservation"
 )
 
@@ -22,22 +20,6 @@ func ObserveConsoleURL(genericlisters configobserver.Listers, recorder events.Re
 	listers := genericlisters.(configobservation.Listers)
 	errs := []error{}
 
-	clusterVersionConfig, err := listers.ClusterVersionLister.Get("version")
-	if err != nil {
-		return existingConfig, append(errs, err)
-	}
-
-	isConsoleCapabilityEnabled := false
-	for _, capability := range clusterVersionConfig.Status.Capabilities.EnabledCapabilities {
-		if capability == configv1.ClusterVersionCapabilityConsole {
-			isConsoleCapabilityEnabled = true
-			break
-		}
-	}
-	if !isConsoleCapabilityEnabled {
-		return existingConfig, nil
-	}
-
 	consoleConfig, err := listers.ConsoleLister.Get("cluster")
 	if err != nil {
 		return existingConfig, append(errs, err)

pkg/controllers/configobservation/console/observe_consoleurl_test.go

@@ -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]) {
 					t.Errorf("ObserveConsoleURL() errs = %v, want %v", errs, tt.expectedErrs)
 				}
 			}

pkg/controllers/termination/termination_controller.go

@@ -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 {
+	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
+}

pkg/operator/starter.go

@@ -74,6 +74,7 @@ import (
 	"github.com/openshift/cluster-authentication-operator/pkg/controllers/readiness"
 	"github.com/openshift/cluster-authentication-operator/pkg/controllers/routercerts"
 	"github.com/openshift/cluster-authentication-operator/pkg/controllers/serviceca"
+	"github.com/openshift/cluster-authentication-operator/pkg/controllers/termination"
 	"github.com/openshift/cluster-authentication-operator/pkg/controllers/trustdistribution"
 	"github.com/openshift/cluster-authentication-operator/pkg/controllers/webhookauthenticator"
 	"github.com/openshift/cluster-authentication-operator/pkg/operator/assets"
@@ -105,7 +106,7 @@ type operatorContext struct {
 }
 
 // RunOperator prepares and runs both operators OAuth and OAuthAPIServer
-// TODO: in the future we might move each operator to its onw pkg
+// TODO: in the future we might move each operator to its own pkg
 // TODO: consider using the new operator framework
 func RunOperator(ctx context.Context, controllerContext *controllercmd.ControllerContext) error {
 	kubeClient, err := kubernetes.NewForConfig(controllerContext.ProtoKubeConfig)
@@ -388,6 +389,17 @@ func prepareOauthOperator(ctx context.Context, controllerContext *controllercmd.
 		operatorCtx.kubeInformersForNamespaces.InformersFor("").Core().V1().Nodes(),
 	)
 
+	if !enabledClusterCapabilities.Has("Console") {
+		// This controller is only necessary if the console capability is not yet enabled in the cluster.
+		// Once the console capability is enabled, this controller will restart the auth operator and next
+		// time it comes up, the console cap will already be enabled and this controller won't be added.
+		terminationController := termination.NewTerminationController(
+			operatorCtx.operatorConfigInformer,
+			controllerContext.EventRecorder,
+		)
+		operatorCtx.controllersToRunFunc = append(operatorCtx.controllersToRunFunc, terminationController.Run)
+	}
+
 	systemCABundle, err := loadSystemCACertBundle()
 	if err != nil {
 		return err