From eab7a9323ec4f708485c0413c56a08352ca12380 Mon Sep 17 00:00:00 2001 From: Vadim Rutkovsky Date: Mon, 12 Aug 2024 09:14:26 +0200 Subject: [PATCH] certrotationcontroller: use minutes instead of days when FeatureShortCertRotation is enabled --- pkg/cmd/recoverycontroller/cmd.go | 29 +++++++++++---- .../certrotationcontroller.go | 37 +++++++++++-------- pkg/operator/starter.go | 11 +----- 3 files changed, 44 insertions(+), 33 deletions(-) diff --git a/pkg/cmd/recoverycontroller/cmd.go b/pkg/cmd/recoverycontroller/cmd.go index 947ead6ff..b1d9076d8 100644 --- a/pkg/cmd/recoverycontroller/cmd.go +++ b/pkg/cmd/recoverycontroller/cmd.go @@ -3,15 +3,19 @@ package recoverycontroller import ( "context" "fmt" + "time" operatorv1 "github.com/openshift/api/operator/v1" + configeversionedclient "github.com/openshift/client-go/config/clientset/versioned" + configexternalinformers "github.com/openshift/client-go/config/informers/externalversions" "github.com/openshift/cluster-kube-controller-manager-operator/pkg/operator" "github.com/openshift/cluster-kube-controller-manager-operator/pkg/operator/certrotationcontroller" "github.com/openshift/cluster-kube-controller-manager-operator/pkg/operator/operatorclient" "github.com/openshift/cluster-kube-controller-manager-operator/pkg/version" "github.com/openshift/library-go/pkg/controller/controllercmd" - "github.com/openshift/library-go/pkg/operator/certrotation" + "github.com/openshift/library-go/pkg/operator/configobserver/featuregates" "github.com/openshift/library-go/pkg/operator/genericoperatorclient" + "github.com/openshift/library-go/pkg/operator/status" "github.com/openshift/library-go/pkg/operator/v1helpers" "github.com/spf13/cobra" "k8s.io/client-go/kubernetes" @@ -91,10 +95,20 @@ func (o *Options) Run(ctx context.Context) error { return err } - certRotationScale, err := certrotation.GetCertRotationScale(ctx, kubeClient, operatorclient.GlobalUserSpecifiedConfigNamespace) + configClient, err := configeversionedclient.NewForConfig(o.controllerContext.KubeConfig) if err != nil { - return err + return fmt.Errorf("failed to create config client: %w", err) } + configInformers := configexternalinformers.NewSharedInformerFactory(configClient, 10*time.Minute) + desiredVersion := status.VersionForOperatorFromEnv() + missingVersion := "0.0.1-snapshot" + featureGateAccessor := featuregates.NewFeatureGateAccess( + desiredVersion, missingVersion, + configInformers.Config().V1().ClusterVersions(), configInformers.Config().V1().FeatureGates(), + o.controllerContext.EventRecorder, + ) + go featureGateAccessor.Run(ctx) + go configInformers.Start(ctx.Done()) certRotationController, err := certrotationcontroller.NewCertRotationControllerOnlyWhenExpired( v1helpers.CachedSecretGetter(kubeClient.CoreV1(), kubeInformersForNamespaces), @@ -102,10 +116,7 @@ func (o *Options) Run(ctx context.Context) error { operatorClient, kubeInformersForNamespaces, o.controllerContext.EventRecorder, - // this is weird, but when we turn down rotation in CI, we go fast enough that kubelets and kas are racing to observe the new signer before the signer is used. - // we need to establish some kind of delay or back pressure to prevent the rollout. This ensures we don't trigger kas restart - // during e2e tests for now. - certRotationScale*8, + featureGateAccessor, ) if err != nil { return err @@ -134,6 +145,10 @@ func (o *Options) Run(ctx context.Context) error { csrController.Run(ctx) }() + go func() { + featureGateAccessor.Run(ctx) + }() + <-ctx.Done() return nil diff --git a/pkg/operator/certrotationcontroller/certrotationcontroller.go b/pkg/operator/certrotationcontroller/certrotationcontroller.go index f4a6954ed..05448dcea 100644 --- a/pkg/operator/certrotationcontroller/certrotationcontroller.go +++ b/pkg/operator/certrotationcontroller/certrotationcontroller.go @@ -2,18 +2,19 @@ package certrotationcontroller import ( "context" + "fmt" "time" - "k8s.io/klog/v2" - corev1client "k8s.io/client-go/kubernetes/typed/core/v1" "github.com/openshift/library-go/pkg/operator/certrotation" "github.com/openshift/library-go/pkg/operator/events" "github.com/openshift/library-go/pkg/operator/v1helpers" + features "github.com/openshift/api/features" "github.com/openshift/cluster-kube-controller-manager-operator/pkg/operator/operatorclient" "github.com/openshift/library-go/pkg/controller/factory" + "github.com/openshift/library-go/pkg/operator/configobserver/featuregates" ) // defaultRotationDay is the default rotation base for all cert rotation operations. @@ -29,7 +30,7 @@ func NewCertRotationController( operatorClient v1helpers.StaticPodOperatorClient, kubeInformersForNamespaces v1helpers.KubeInformersForNamespaces, eventRecorder events.Recorder, - day time.Duration, + featureGateAccessor featuregates.FeatureGateAccess, ) (*CertRotationController, error) { return newCertRotationController( secretsGetter, @@ -37,7 +38,7 @@ func NewCertRotationController( operatorClient, kubeInformersForNamespaces, eventRecorder, - day, + featureGateAccessor, false, ) } @@ -48,7 +49,7 @@ func NewCertRotationControllerOnlyWhenExpired( operatorClient v1helpers.StaticPodOperatorClient, kubeInformersForNamespaces v1helpers.KubeInformersForNamespaces, eventRecorder events.Recorder, - day time.Duration, + featureGateAccessor featuregates.FeatureGateAccess, ) (*CertRotationController, error) { return newCertRotationController( secretsGetter, @@ -56,7 +57,7 @@ func NewCertRotationControllerOnlyWhenExpired( operatorClient, kubeInformersForNamespaces, eventRecorder, - day, + featureGateAccessor, true, ) } @@ -67,16 +68,20 @@ func newCertRotationController( operatorClient v1helpers.StaticPodOperatorClient, kubeInformersForNamespaces v1helpers.KubeInformersForNamespaces, eventRecorder events.Recorder, - day time.Duration, + featureGateAccessor featuregates.FeatureGateAccess, refreshOnlyWhenExpired bool, ) (*CertRotationController, error) { ret := &CertRotationController{} - rotationDay := defaultRotationDay - if day != time.Duration(0) { - rotationDay = day - klog.Warningf("!!! UNSUPPORTED VALUE SET !!!") - klog.Warningf("Certificate rotation base set to %q", rotationDay) + monthPeriod := time.Hour * 24 * 30 + + featureGates, err := featureGateAccessor.CurrentFeatureGates() + if err != nil { + return nil, fmt.Errorf("unable to get FeatureGates: %w", err) + } + + if featureGates.Enabled(features.FeatureShortCertRotation) { + monthPeriod = time.Hour } certRotator := certrotation.NewCertRotationController( @@ -88,8 +93,8 @@ func newCertRotationController( AdditionalAnnotations: certrotation.AdditionalAnnotations{ JiraComponent: "kube-controller-manager", }, - Validity: 60 * rotationDay, - Refresh: 30 * rotationDay, + Validity: monthPeriod * 2, + Refresh: monthPeriod, RefreshOnlyWhenExpired: refreshOnlyWhenExpired, Informer: kubeInformersForNamespaces.InformersFor(operatorclient.OperatorNamespace).Core().V1().Secrets(), Lister: kubeInformersForNamespaces.InformersFor(operatorclient.OperatorNamespace).Core().V1().Secrets().Lister(), @@ -113,8 +118,8 @@ func newCertRotationController( AdditionalAnnotations: certrotation.AdditionalAnnotations{ JiraComponent: "kube-controller-manager", }, - Validity: 30 * rotationDay, - Refresh: 15 * rotationDay, + Validity: monthPeriod, + Refresh: monthPeriod / 2, RefreshOnlyWhenExpired: refreshOnlyWhenExpired, CertCreator: &certrotation.SignerRotation{ SignerName: "kube-csr-signer", diff --git a/pkg/operator/starter.go b/pkg/operator/starter.go index a13c29618..21f61cfee 100644 --- a/pkg/operator/starter.go +++ b/pkg/operator/starter.go @@ -21,7 +21,6 @@ import ( "github.com/openshift/cluster-kube-controller-manager-operator/pkg/operator/resourcesynccontroller" "github.com/openshift/cluster-kube-controller-manager-operator/pkg/operator/targetconfigcontroller" "github.com/openshift/library-go/pkg/controller/controllercmd" - "github.com/openshift/library-go/pkg/operator/certrotation" "github.com/openshift/library-go/pkg/operator/configobserver/featuregates" "github.com/openshift/library-go/pkg/operator/genericoperatorclient" "github.com/openshift/library-go/pkg/operator/latencyprofilecontroller" @@ -260,21 +259,13 @@ func RunOperator(ctx context.Context, cc *controllercmd.ControllerContext) error cc.EventRecorder, ) - certRotationScale, err := certrotation.GetCertRotationScale(ctx, kubeClient, operatorclient.GlobalUserSpecifiedConfigNamespace) - if err != nil { - return err - } - certRotationController, err := certrotationcontroller.NewCertRotationController( v1helpers.CachedSecretGetter(kubeClient.CoreV1(), kubeInformersForNamespaces), v1helpers.CachedConfigMapGetter(kubeClient.CoreV1(), kubeInformersForNamespaces), operatorClient, kubeInformersForNamespaces, cc.EventRecorder, - // this is weird, but when we turn down rotation in CI, we go fast enough that kubelets and kas are racing to observe the new signer before the signer is used. - // we need to establish some kind of delay or back pressure to prevent the rollout. This ensures we don't trigger kas restart - // during e2e tests for now. - certRotationScale*8, + featureGateAccessor, ) if err != nil { return err