From 2a65eb04a0eac6844434f0fedc8fc6cdec5b33bc Mon Sep 17 00:00:00 2001 From: Neil Girard Date: Mon, 17 Aug 2026 12:56:37 -0400 Subject: [PATCH] Moved node sync job creation from manifest to operator controller --- cmd/config-sync-controllers/main.go | 34 ++- cmd/node-label-sync-job/main.go | 8 +- ...ler-manager-operator_02_rbac_operator.yaml | 11 + ...roller-manager-operator_50_deployment.yaml | 2 + ...ud-controller-manager-operator_00_job.yaml | 112 --------- pkg/controllers/common_consts.go | 6 + .../node_label_sync_job_controller.go | 232 ++++++++++++++++++ .../node_label_sync_job_controller_test.go | 152 ++++++++++++ pkg/controllers/vsphere_node_label_sync.go | 6 +- pkg/controllers/watch_predicates.go | 23 ++ pkg/restmapper/predicates.go | 6 + 11 files changed, 472 insertions(+), 120 deletions(-) delete mode 100644 manifests/0000_90_cloud-controller-manager-operator_00_job.yaml create mode 100644 pkg/controllers/node_label_sync_job_controller.go create mode 100644 pkg/controllers/node_label_sync_job_controller_test.go diff --git a/cmd/config-sync-controllers/main.go b/cmd/config-sync-controllers/main.go index bac97ca7f..dac148af1 100644 --- a/cmd/config-sync-controllers/main.go +++ b/cmd/config-sync-controllers/main.go @@ -29,6 +29,7 @@ import ( "k8s.io/client-go/kubernetes" _ "k8s.io/client-go/plugin/pkg/client/auth" + batchv1 "k8s.io/api/batch/v1" "k8s.io/apimachinery/pkg/runtime" utilruntime "k8s.io/apimachinery/pkg/util/runtime" clientgoscheme "k8s.io/client-go/kubernetes/scheme" @@ -37,6 +38,7 @@ import ( "k8s.io/utils/clock" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/cache" + "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/healthz" metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server" @@ -99,6 +101,12 @@ func main() { ctrl.SetLogger(klog.NewKlogr().WithName("CCCMOConfigSyncControllers")) + operatorImage := os.Getenv("OPERATOR_IMAGE") + if operatorImage == "" { + setupLog.Error(nil, "OPERATOR_IMAGE environment variable is required") + os.Exit(1) + } + restConfig := ctrl.GetConfigOrDie() le := util.GetLeaderElectionDefaults(restConfig, configv1.LeaderElection{ Disable: !leaderElectionConfig.LeaderElect, @@ -114,7 +122,19 @@ func main() { DefaultNamespaces: map[string]cache.Config{ *managedNamespace: {}, controllers.OpenshiftConfigNamespace: {}, - controllers.OpenshiftManagedConfigNamespace: {}}, + controllers.OpenshiftManagedConfigNamespace: {}, + }, + // The node-label-sync Job only ever lives in the operator's own namespace. Without this + // override, the multi-namespace cache built from DefaultNamespaces above would otherwise + // try to list/watch Jobs in every one of those namespaces too, which the operator has no + // RBAC for. + ByObject: map[client.Object]cache.ByObject{ + &batchv1.Job{}: { + Namespaces: map[string]cache.Config{ + controllers.OperatorNamespace: {}, + }, + }, + }, } mgr, err := ctrl.NewManager(restConfig, ctrl.Options{ @@ -126,6 +146,7 @@ func main() { MapperProvider: restmapper.NewPartialRestMapperProvider( restmapper.Or( restmapper.KubernetesCoreGroup, + restmapper.KubernetesBatchGroup, restmapper.OpenshiftOperatorGroup, restmapper.OpenshiftConfigGroup, ), @@ -202,6 +223,17 @@ func main() { setupLog.Error(err, "unable to create Trusted CA sync controller", "controller", "ClusterOperator") os.Exit(1) } + + if err = (&controllers.NodeLabelSyncJobReconciler{ + Client: mgr.GetClient(), + Namespace: controllers.OperatorNamespace, + Image: operatorImage, + ReleaseVersion: controllers.GetReleaseVersion(), + FeatureGateAccess: featureGateAccessor, + }).SetupWithManager(mgr); err != nil { + setupLog.Error(err, "unable to create node-label-sync Job controller", "controller", "NodeLabelSyncJob") + os.Exit(1) + } // +kubebuilder:scaffold:builder if err := mgr.AddHealthzCheck("health", healthz.Ping); err != nil { diff --git a/cmd/node-label-sync-job/main.go b/cmd/node-label-sync-job/main.go index 329780371..e33f1e92e 100644 --- a/cmd/node-label-sync-job/main.go +++ b/cmd/node-label-sync-job/main.go @@ -15,10 +15,10 @@ limitations under the License. */ // Command node-label-sync-job runs to completion once, backfilling the -// node.openshift.io/platform-type=vsphere label onto vSphere nodes that are missing it. It is -// installed as a Kubernetes Job by the CVO (see -// manifests/0000_90_cloud-controller-manager-operator_00_job.yaml) rather -// than run as an ongoing in-process controller. +// node.openshift.io/platform-type=vsphere label onto vSphere nodes that are missing it. The +// operator's own NodeLabelSyncJobReconciler (see +// pkg/controllers/node_label_sync_job_controller.go) creates this as a Kubernetes Job on demand, +// rather than running it as an ongoing in-process controller. package main import ( diff --git a/manifests/0000_26_cloud-controller-manager-operator_02_rbac_operator.yaml b/manifests/0000_26_cloud-controller-manager-operator_02_rbac_operator.yaml index 91eeb834b..c5ba86502 100644 --- a/manifests/0000_26_cloud-controller-manager-operator_02_rbac_operator.yaml +++ b/manifests/0000_26_cloud-controller-manager-operator_02_rbac_operator.yaml @@ -232,6 +232,17 @@ rules: - list - patch + # For the operator to create (and never update) the one-shot node-label-sync Job. + - apiGroups: + - batch + resources: + - jobs + verbs: + - get + - list + - watch + - create + --- apiVersion: rbac.authorization.k8s.io/v1 kind: Role diff --git a/manifests/0000_26_cloud-controller-manager-operator_50_deployment.yaml b/manifests/0000_26_cloud-controller-manager-operator_50_deployment.yaml index aa13a39e6..308b6b45e 100644 --- a/manifests/0000_26_cloud-controller-manager-operator_50_deployment.yaml +++ b/manifests/0000_26_cloud-controller-manager-operator_50_deployment.yaml @@ -107,6 +107,8 @@ spec: env: - name: RELEASE_VERSION value: "0.0.1-snapshot" + - name: OPERATOR_IMAGE + value: quay.io/openshift/origin-cluster-cloud-controller-manager-operator resources: requests: cpu: 10m diff --git a/manifests/0000_90_cloud-controller-manager-operator_00_job.yaml b/manifests/0000_90_cloud-controller-manager-operator_00_job.yaml deleted file mode 100644 index 24d3f7744..000000000 --- a/manifests/0000_90_cloud-controller-manager-operator_00_job.yaml +++ /dev/null @@ -1,112 +0,0 @@ -apiVersion: batch/v1 -kind: Job -metadata: - name: node-label-sync - namespace: openshift-cloud-controller-manager-operator - annotations: - capability.openshift.io/name: CloudControllerManager - include.release.openshift.io/self-managed-high-availability: "true" - include.release.openshift.io/single-node-developer: "true" - # Only create this Job on clusters where the VSphereMixedNodeEnv feature gate is enabled, - # matching the same gate the vSphere CCM checks before applying the node.openshift.io/platform-type - # label at node-init time (see pkg/cloud/vsphere/vsphere.go). The CVO re-evaluates this against the - # live FeatureGate object on every sync, so the Job is created as soon as the gate turns on. - release.openshift.io/feature-gate: "VSphereMixedNodeEnv" - # This Job must only ever be created once, never updated. Its pod template's container image - # changes on nearly every release, but a Job's pod template is immutable once created, so if - # the CVO ever tried to reconcile this Job in place (instead of only creating it) the update - # would fail on every subsequent upgrade after the first. create-only makes the CVO create it - # once and leave it alone from then on, matching the intended "backfill once" semantics. - release.openshift.io/create-only: "true" - labels: - k8s-app: node-label-sync -spec: - backoffLimit: 6 - activeDeadlineSeconds: 300 - template: - metadata: - labels: - k8s-app: node-label-sync - annotations: - target.workload.openshift.io/management: '{"effect": "PreferredDuringScheduling"}' - openshift.io/required-scc: hostaccess - spec: - priorityClassName: system-node-critical - serviceAccountName: cluster-cloud-controller-manager - restartPolicy: OnFailure - # This Job may run before pod networking (CNI/OVN service routing) is functional on a given - # node -- e.g. right after a control-plane node reboots during an upgrade. hostNetwork plus - # sourcing /etc/kubernetes/apiserver-url.env (mirroring the operator Deployment in - # 0000_26_cloud-controller-manager-operator_50_deployment.yaml) lets the container reach the - # API server directly instead of through the "kubernetes" Service ClusterIP, which requires - # kube-proxy/OVN to already be wired up. That file is only populated on control-plane nodes, - # hence the master nodeSelector/tolerations. - hostNetwork: true - nodeSelector: - node-role.kubernetes.io/master: "" - tolerations: - - key: "node-role.kubernetes.io/master" - operator: "Exists" - effect: "NoSchedule" - - key: "node.kubernetes.io/unreachable" - operator: "Exists" - effect: "NoExecute" - tolerationSeconds: 120 - - key: "node.kubernetes.io/not-ready" - operator: "Exists" - effect: "NoExecute" - tolerationSeconds: 120 - - key: "node.cloudprovider.kubernetes.io/uninitialized" - operator: "Exists" - effect: "NoSchedule" - # CNI relies on CCM to fill in IP information on Node objects. - # Therefore we must schedule before the CNI can mark the Node as ready. - - key: "node.kubernetes.io/not-ready" - operator: "Exists" - effect: "NoSchedule" - containers: - - name: node-label-sync - image: quay.io/openshift/origin-cluster-cloud-controller-manager-operator - command: - - /bin/bash - - -c - - | - #!/bin/bash - set -o allexport - if [[ -f /etc/kubernetes/apiserver-url.env ]]; then - source /etc/kubernetes/apiserver-url.env - else - URL_ONLY_KUBECONFIG=/etc/kubernetes/kubeconfig - fi - exec /node-label-sync-job - env: - - name: RELEASE_VERSION - value: "0.0.1-snapshot" - resources: - requests: - cpu: 10m - memory: 50Mi - terminationMessagePolicy: FallbackToLogsOnError - # runAsNonRoot is deliberately NOT set here: this image (like the operator's own - # Deployment container, which also runs unguarded) has no USER directive in its - # Dockerfile and defaults to root, and the hostaccess SCC does not reliably assign - # a non-root UID for this pod. Setting runAsNonRoot: true without a matching - # runAsUser makes the kubelet refuse to start the container at all ("container has - # runAsNonRoot and image will run as root"), which is what happened in CI (PR #497, - # e2e-azure-ovn-upgrade): the container crash-looped for the full activeDeadlineSeconds - # window and tripped the "events should not repeat pathologically" invariant test. - securityContext: - allowPrivilegeEscalation: false - readOnlyRootFilesystem: true - capabilities: - drop: - - ALL - volumeMounts: - - mountPath: /etc/kubernetes - name: host-etc-kube - readOnly: true - volumes: - - name: host-etc-kube - hostPath: - path: /etc/kubernetes - type: Directory diff --git a/pkg/controllers/common_consts.go b/pkg/controllers/common_consts.go index d91937a3b..678ea2651 100644 --- a/pkg/controllers/common_consts.go +++ b/pkg/controllers/common_consts.go @@ -3,6 +3,10 @@ package controllers const ( DefaultManagedNamespace = "openshift-cloud-controller-manager" + // OperatorNamespace is the namespace the operator's own Deployment (and the + // resources it creates directly, e.g. the node-label-sync Job) run in. + OperatorNamespace = "openshift-cloud-controller-manager-operator" + infrastructureResourceName = "cluster" OpenshiftConfigNamespace = "openshift-config" @@ -10,5 +14,7 @@ const ( syncedCloudConfigMapName = "cloud-conf" + nodeLabelSyncJobName = "node-label-sync" + proxyResourceName = "cluster" ) diff --git a/pkg/controllers/node_label_sync_job_controller.go b/pkg/controllers/node_label_sync_job_controller.go new file mode 100644 index 000000000..7ef3da612 --- /dev/null +++ b/pkg/controllers/node_label_sync_job_controller.go @@ -0,0 +1,232 @@ +package controllers + +import ( + "context" + "fmt" + + configv1 "github.com/openshift/api/config/v1" + "github.com/openshift/api/features" + "github.com/openshift/library-go/pkg/operator/configobserver/featuregates" + batchv1 "k8s.io/api/batch/v1" + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/resource" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/klog/v2" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/builder" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/handler" + "sigs.k8s.io/controller-runtime/pkg/reconcile" +) + +const ( + nodeLabelSyncJobActiveDeadlineSeconds int64 = 300 + nodeLabelSyncJobBackoffLimit int32 = 6 +) + +// NodeLabelSyncJobReconciler ensures the node-label-sync Job exists on vSphere clusters with the +// VSphereMixedNodeEnv feature gate enabled, creating it if it's missing. It never updates or +// deletes the Job once created: the Job's pod template is immutable, so this mirrors the +// "backfill once" semantics that a CVO-installed manifest would get from the +// release.openshift.io/create-only annotation, without going through CVO's manifest-apply path. +type NodeLabelSyncJobReconciler struct { + client.Client + // Namespace is where the node-label-sync Job is created. Set to controllers.OperatorNamespace + // in production; tests may point it at a dedicated test namespace. + Namespace string + // Image is the container image stamped onto the Job, read from the OPERATOR_IMAGE env + // variable, which carries the same resolved pullspec as the running operator image. + Image string + // ReleaseVersion is stamped onto the Job's RELEASE_VERSION env variable. + ReleaseVersion string + FeatureGateAccess featuregates.FeatureGateAccess +} + +func (r *NodeLabelSyncJobReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { + infra := &configv1.Infrastructure{} + if err := r.Get(ctx, client.ObjectKey{Name: infrastructureResourceName}, infra); err != nil { + if apierrors.IsNotFound(err) { + klog.Infof("infrastructure resource not found, skipping node-label-sync Job creation") + return ctrl.Result{}, nil + } + return ctrl.Result{}, fmt.Errorf("failed to get infrastructure: %w", err) + } + + if infra.Status.PlatformStatus == nil || infra.Status.PlatformStatus.Type != configv1.VSpherePlatformType { + klog.V(2).Infof("platform is not vSphere, skipping node-label-sync Job creation") + return ctrl.Result{}, nil + } + + if r.FeatureGateAccess == nil { + return ctrl.Result{}, reconcile.TerminalError(fmt.Errorf("FeatureGateAccess is not configured")) + } + + currentFeatureGates, err := r.FeatureGateAccess.CurrentFeatureGates() + if err != nil { + return ctrl.Result{}, fmt.Errorf("failed to get current feature gates: %w", err) + } + if !currentFeatureGates.Enabled(features.FeatureGateVSphereMixedNodeEnv) { + klog.V(2).Infof("%s feature gate is disabled, skipping node-label-sync Job creation", features.FeatureGateVSphereMixedNodeEnv) + return ctrl.Result{}, nil + } + + existing := &batchv1.Job{} + err = r.Get(ctx, client.ObjectKey{Name: nodeLabelSyncJobName, Namespace: r.Namespace}, existing) + if err == nil { + klog.V(2).Infof("node-label-sync Job already exists, leaving it alone") + return ctrl.Result{}, nil + } + if !apierrors.IsNotFound(err) { + return ctrl.Result{}, fmt.Errorf("failed to get node-label-sync Job: %w", err) + } + + job := r.buildNodeLabelSyncJob() + if err := r.Create(ctx, job); err != nil && !apierrors.IsAlreadyExists(err) { + return ctrl.Result{}, fmt.Errorf("failed to create node-label-sync Job: %w", err) + } + + klog.Infof("created node-label-sync Job") + return ctrl.Result{}, nil +} + +func (r *NodeLabelSyncJobReconciler) buildNodeLabelSyncJob() *batchv1.Job { + activeDeadlineSeconds := nodeLabelSyncJobActiveDeadlineSeconds + backoffLimit := nodeLabelSyncJobBackoffLimit + allowPrivilegeEscalation := false + readOnlyRootFilesystem := true + hostPathDirectory := corev1.HostPathDirectory + + return &batchv1.Job{ + ObjectMeta: metav1.ObjectMeta{ + Name: nodeLabelSyncJobName, + Namespace: r.Namespace, + Labels: map[string]string{ + "k8s-app": nodeLabelSyncJobName, + }, + }, + Spec: batchv1.JobSpec{ + BackoffLimit: &backoffLimit, + ActiveDeadlineSeconds: &activeDeadlineSeconds, + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + "k8s-app": nodeLabelSyncJobName, + }, + Annotations: map[string]string{ + "target.workload.openshift.io/management": `{"effect": "PreferredDuringScheduling"}`, + "openshift.io/required-scc": "hostaccess", + }, + }, + Spec: corev1.PodSpec{ + PriorityClassName: "system-node-critical", + ServiceAccountName: "cluster-cloud-controller-manager", + RestartPolicy: corev1.RestartPolicyOnFailure, + // This Job may run before pod networking (CNI/OVN service routing) is functional + // on a given node -- e.g. right after a control-plane node reboots during an + // upgrade. hostNetwork plus sourcing /etc/kubernetes/apiserver-url.env (mirroring + // the operator Deployment) lets the container reach the API server directly + // instead of through the "kubernetes" Service ClusterIP, which requires + // kube-proxy/OVN to already be wired up. That file is only populated on + // control-plane nodes, hence the master nodeSelector/tolerations. + HostNetwork: true, + NodeSelector: map[string]string{ + "node-role.kubernetes.io/master": "", + }, + Tolerations: []corev1.Toleration{ + {Key: "node-role.kubernetes.io/master", Operator: corev1.TolerationOpExists, Effect: corev1.TaintEffectNoSchedule}, + {Key: "node.kubernetes.io/unreachable", Operator: corev1.TolerationOpExists, Effect: corev1.TaintEffectNoExecute, TolerationSeconds: int64Ptr(120)}, + {Key: "node.kubernetes.io/not-ready", Operator: corev1.TolerationOpExists, Effect: corev1.TaintEffectNoExecute, TolerationSeconds: int64Ptr(120)}, + {Key: "node.cloudprovider.kubernetes.io/uninitialized", Operator: corev1.TolerationOpExists, Effect: corev1.TaintEffectNoSchedule}, + // CNI relies on CCM to fill in IP information on Node objects. + // Therefore we must schedule before the CNI can mark the Node as ready. + {Key: "node.kubernetes.io/not-ready", Operator: corev1.TolerationOpExists, Effect: corev1.TaintEffectNoSchedule}, + }, + Containers: []corev1.Container{ + { + Name: nodeLabelSyncJobName, + Image: r.Image, + Command: []string{ + "/bin/bash", + "-c", + `#!/bin/bash +set -o allexport +if [[ -f /etc/kubernetes/apiserver-url.env ]]; then + source /etc/kubernetes/apiserver-url.env +else + URL_ONLY_KUBECONFIG=/etc/kubernetes/kubeconfig +fi +exec /node-label-sync-job +`, + }, + Env: []corev1.EnvVar{ + {Name: "RELEASE_VERSION", Value: r.ReleaseVersion}, + }, + Resources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("10m"), + corev1.ResourceMemory: resource.MustParse("50Mi"), + }, + }, + TerminationMessagePolicy: corev1.TerminationMessageFallbackToLogsOnError, + // runAsNonRoot is deliberately NOT set here: this image (like the + // operator's own Deployment container, which also runs unguarded) has no + // USER directive in its Dockerfile and defaults to root, and the + // hostaccess SCC does not reliably assign a non-root UID for this pod. + // Setting runAsNonRoot: true without a matching runAsUser makes the + // kubelet refuse to start the container at all ("container has + // runAsNonRoot and image will run as root"). + SecurityContext: &corev1.SecurityContext{ + AllowPrivilegeEscalation: &allowPrivilegeEscalation, + ReadOnlyRootFilesystem: &readOnlyRootFilesystem, + Capabilities: &corev1.Capabilities{ + Drop: []corev1.Capability{"ALL"}, + }, + }, + VolumeMounts: []corev1.VolumeMount{ + {Name: "host-etc-kube", MountPath: "/etc/kubernetes", ReadOnly: true}, + }, + }, + }, + Volumes: []corev1.Volume{ + { + Name: "host-etc-kube", + VolumeSource: corev1.VolumeSource{ + HostPath: &corev1.HostPathVolumeSource{ + Path: "/etc/kubernetes", + Type: &hostPathDirectory, + }, + }, + }, + }, + }, + }, + }, + } +} + +// SetupWithManager sets up the controller with the Manager. +func (r *NodeLabelSyncJobReconciler) SetupWithManager(mgr ctrl.Manager) error { + build := ctrl.NewControllerManagedBy(mgr). + Named("NodeLabelSyncJobController"). + For( + &batchv1.Job{}, + builder.WithPredicates(nodeLabelSyncJobPredicate(r.Namespace)), + ). + Watches( + &configv1.Infrastructure{}, + handler.EnqueueRequestsFromMapFunc(toNodeLabelSyncJob(r.Namespace)), + builder.WithPredicates(infrastructurePredicates()), + ). + Watches( + &configv1.FeatureGate{}, + handler.EnqueueRequestsFromMapFunc(toNodeLabelSyncJob(r.Namespace)), + builder.WithPredicates(featureGatePredicates()), + ) + + return build.Complete(r) +} + +func int64Ptr(v int64) *int64 { + return &v +} diff --git a/pkg/controllers/node_label_sync_job_controller_test.go b/pkg/controllers/node_label_sync_job_controller_test.go new file mode 100644 index 000000000..2061a2561 --- /dev/null +++ b/pkg/controllers/node_label_sync_job_controller_test.go @@ -0,0 +1,152 @@ +package controllers + +import ( + "context" + "fmt" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + configv1 "github.com/openshift/api/config/v1" + "github.com/openshift/api/features" + "github.com/openshift/library-go/pkg/operator/configobserver/featuregates" + batchv1 "k8s.io/api/batch/v1" + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/reconcile" +) + +const ( + testNodeLabelSyncNamespace = "test-node-label-sync" + testNodeLabelSyncImage = "quay.io/example/test-node-label-sync-job:latest" + testNodeLabelSyncVersion = "1.2.3-test" +) + +// deleteJob deletes the Job and waits for it to disappear. envtest has no job controller +// running to remove the batch.kubernetes.io/job-tracking finalizer that the apiserver adds on +// create, so a plain Delete would otherwise leave the object stuck terminating forever and +// block the next test from creating a Job with the same name. +func deleteJob(ctx context.Context, key client.ObjectKey) { + job := &batchv1.Job{} + if err := cl.Get(ctx, key, job); apierrors.IsNotFound(err) { + return + } + _ = cl.Delete(ctx, job) + + Eventually(func() error { + j := &batchv1.Job{} + if err := cl.Get(ctx, key, j); err != nil { + return err + } + if len(j.Finalizers) > 0 { + j.Finalizers = nil + _ = cl.Update(ctx, j) + } + return fmt.Errorf("job %s still exists", key) + }).Should(MatchError(apierrors.IsNotFound, "IsNotFound")) +} + +var _ = Describe("NodeLabelSyncJobReconciler", func() { + ctx := context.Background() + + jobKey := client.ObjectKey{Name: nodeLabelSyncJobName, Namespace: testNodeLabelSyncNamespace} + + newReconciler := func(enabled, disabled []configv1.FeatureGateName) *NodeLabelSyncJobReconciler { + return &NodeLabelSyncJobReconciler{ + Client: cl, + Namespace: testNodeLabelSyncNamespace, + Image: testNodeLabelSyncImage, + ReleaseVersion: testNodeLabelSyncVersion, + FeatureGateAccess: featuregates.NewHardcodedFeatureGateAccessForTesting(enabled, disabled, nil, nil), + } + } + + createInfra := func(platform configv1.PlatformType) { + infra := makeInfrastructureResource(platform) + Expect(cl.Create(ctx, infra)).To(Succeed()) + infra.Status = makeInfraStatus(platform) + Expect(cl.Status().Update(ctx, infra)).To(Succeed()) + } + + BeforeEach(func() { + ns := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: testNodeLabelSyncNamespace}} + if err := cl.Create(ctx, ns); err != nil && !apierrors.IsAlreadyExists(err) { + Expect(err).NotTo(HaveOccurred()) + } + }) + + AfterEach(func() { + infra := &configv1.Infrastructure{ObjectMeta: metav1.ObjectMeta{Name: infrastructureResourceName}} + _ = cl.Delete(ctx, infra) + Eventually(func() error { + return cl.Get(ctx, client.ObjectKeyFromObject(infra), &configv1.Infrastructure{}) + }).Should(MatchError(apierrors.IsNotFound, "IsNotFound")) + + deleteJob(ctx, jobKey) + }) + + It("does nothing when Infrastructure does not exist", func() { + reconciler := newReconciler([]configv1.FeatureGateName{features.FeatureGateVSphereMixedNodeEnv}, nil) + _, err := reconciler.Reconcile(ctx, reconcile.Request{}) + Expect(err).NotTo(HaveOccurred()) + + err = cl.Get(ctx, jobKey, &batchv1.Job{}) + Expect(apierrors.IsNotFound(err)).To(BeTrue()) + }) + + It("does nothing for a non-vSphere platform", func() { + createInfra(configv1.AWSPlatformType) + + reconciler := newReconciler([]configv1.FeatureGateName{features.FeatureGateVSphereMixedNodeEnv}, nil) + _, err := reconciler.Reconcile(ctx, reconcile.Request{}) + Expect(err).NotTo(HaveOccurred()) + + err = cl.Get(ctx, jobKey, &batchv1.Job{}) + Expect(apierrors.IsNotFound(err)).To(BeTrue()) + }) + + It("does nothing when the VSphereMixedNodeEnv feature gate is disabled", func() { + createInfra(configv1.VSpherePlatformType) + + reconciler := newReconciler(nil, []configv1.FeatureGateName{features.FeatureGateVSphereMixedNodeEnv}) + _, err := reconciler.Reconcile(ctx, reconcile.Request{}) + Expect(err).NotTo(HaveOccurred()) + + err = cl.Get(ctx, jobKey, &batchv1.Job{}) + Expect(apierrors.IsNotFound(err)).To(BeTrue()) + }) + + It("creates the Job once vSphere platform and the feature gate are both active", func() { + createInfra(configv1.VSpherePlatformType) + + reconciler := newReconciler([]configv1.FeatureGateName{features.FeatureGateVSphereMixedNodeEnv}, nil) + _, err := reconciler.Reconcile(ctx, reconcile.Request{}) + Expect(err).NotTo(HaveOccurred()) + + job := &batchv1.Job{} + Expect(cl.Get(ctx, jobKey, job)).To(Succeed()) + Expect(job.Spec.Template.Spec.Containers).To(HaveLen(1)) + Expect(job.Spec.Template.Spec.Containers[0].Image).To(Equal(testNodeLabelSyncImage)) + Expect(job.Spec.Template.Spec.Containers[0].Env).To(ContainElement(corev1.EnvVar{Name: "RELEASE_VERSION", Value: testNodeLabelSyncVersion})) + Expect(job.Spec.Template.Spec.HostNetwork).To(BeTrue()) + Expect(job.Spec.Template.Spec.NodeSelector).To(HaveKeyWithValue("node-role.kubernetes.io/master", "")) + }) + + It("leaves an already-existing Job untouched", func() { + createInfra(configv1.VSpherePlatformType) + + reconciler := newReconciler([]configv1.FeatureGateName{features.FeatureGateVSphereMixedNodeEnv}, nil) + + existingJob := reconciler.buildNodeLabelSyncJob() + existingJob.Spec.Template.Spec.Containers[0].Image = "some-other-image" + Expect(cl.Create(ctx, existingJob)).To(Succeed()) + + _, err := reconciler.Reconcile(ctx, reconcile.Request{}) + Expect(err).NotTo(HaveOccurred()) + + job := &batchv1.Job{} + Expect(cl.Get(ctx, jobKey, job)).To(Succeed()) + Expect(job.Spec.Template.Spec.Containers[0].Image).To(Equal("some-other-image")) + }) +}) diff --git a/pkg/controllers/vsphere_node_label_sync.go b/pkg/controllers/vsphere_node_label_sync.go index bd2f11422..4908b8c53 100644 --- a/pkg/controllers/vsphere_node_label_sync.go +++ b/pkg/controllers/vsphere_node_label_sync.go @@ -23,9 +23,9 @@ import ( // clusters can mix vSphere and bare-metal nodes, so each node's spec.providerID is checked to // confirm it is actually a vSphere node before labeling it. // -// This is run to completion once by the node-label-sync-job Job that the CVO installs -// (see manifests/0000_90_cloud-controller-manager-operator_00_job.yaml), -// rather than as an ongoing in-process controller. +// This is run to completion once by the node-label-sync-job Job, which the operator's own +// NodeLabelSyncJobReconciler creates on demand (see node_label_sync_job_controller.go) rather +// than as an ongoing in-process controller. func SyncVSphereNodeLabels(ctx context.Context, c client.Client) error { infra := &configv1.Infrastructure{} if err := c.Get(ctx, client.ObjectKey{Name: infrastructureResourceName}, infra); err != nil { diff --git a/pkg/controllers/watch_predicates.go b/pkg/controllers/watch_predicates.go index 766097d20..add3e73c8 100644 --- a/pkg/controllers/watch_predicates.go +++ b/pkg/controllers/watch_predicates.go @@ -5,6 +5,7 @@ import ( configv1 "github.com/openshift/api/config/v1" operatorv1 "github.com/openshift/api/operator/v1" + batchv1 "k8s.io/api/batch/v1" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -130,6 +131,28 @@ func ccmTrustedCABundleConfigMapPredicates(targetNamespace string) predicate.Fun } } +func toNodeLabelSyncJob(namespace string) func(context.Context, client.Object) []reconcile.Request { + return func(context.Context, client.Object) []reconcile.Request { + return []reconcile.Request{{ + NamespacedName: client.ObjectKey{Name: nodeLabelSyncJobName, Namespace: namespace}, + }} + } +} + +func nodeLabelSyncJobPredicate(targetNamespace string) predicate.Funcs { + isNodeLabelSyncJob := func(obj runtime.Object) bool { + job, ok := obj.(*batchv1.Job) + return ok && job.GetNamespace() == targetNamespace && job.GetName() == nodeLabelSyncJobName + } + + return predicate.Funcs{ + CreateFunc: func(e event.CreateEvent) bool { return isNodeLabelSyncJob(e.Object) }, + UpdateFunc: func(e event.UpdateEvent) bool { return isNodeLabelSyncJob(e.ObjectNew) }, + GenericFunc: func(e event.GenericEvent) bool { return isNodeLabelSyncJob(e.Object) }, + DeleteFunc: func(e event.DeleteEvent) bool { return isNodeLabelSyncJob(e.Object) }, + } +} + // Config maps from 'openshift-config' namespace func openshiftConfigNamespacedPredicate() predicate.Funcs { isTrustedCaConfigMap := func(obj runtime.Object) bool { diff --git a/pkg/restmapper/predicates.go b/pkg/restmapper/predicates.go index 4900b8ff2..811a37841 100644 --- a/pkg/restmapper/predicates.go +++ b/pkg/restmapper/predicates.go @@ -41,6 +41,12 @@ func KubernetesPolicyGroup(group *metav1.APIGroup) bool { return group.Name == "policy" } +// KubernetesBatchGroup checks if APIGroup is the Kubernetes' "batch" group +// Job and CronJob resources are sitting here. +func KubernetesBatchGroup(group *metav1.APIGroup) bool { + return group.Name == "batch" +} + // Or combines passed predicate functions in a way to implement logical OR between them. func Or(predicates ...GroupFilterPredicate) GroupFilterPredicate { return func(g *metav1.APIGroup) bool {