OCPBUGS-100052: Created new job to update vSphere nodes to have vsphere label #497
+453
−2
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,145 @@ | ||
| /* | ||
| Copyright 2026. | ||
|
|
||
| Licensed under the Apache License, Version 2.0 (the "License"); | ||
| you may not use this file except in compliance with the License. | ||
| You may obtain a copy of the License at | ||
|
|
||
| http://www.apache.org/licenses/LICENSE-2.0 | ||
|
|
||
| Unless required by applicable law or agreed to in writing, software | ||
| distributed under the License is distributed on an "AS IS" BASIS, | ||
| WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| See the License for the specific language governing permissions and | ||
| 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. | ||
| package main | ||
|
|
||
| import ( | ||
| "context" | ||
| "errors" | ||
| "flag" | ||
| "os" | ||
| "time" | ||
|
|
||
| "k8s.io/apimachinery/pkg/runtime" | ||
| utilruntime "k8s.io/apimachinery/pkg/util/runtime" | ||
|
|
||
| // Import all Kubernetes client auth plugins (e.g. Azure, GCP, OIDC, etc.) | ||
| // to ensure that exec-entrypoint and run can make use of them. | ||
| "k8s.io/client-go/kubernetes" | ||
| clientgoscheme "k8s.io/client-go/kubernetes/scheme" | ||
| _ "k8s.io/client-go/plugin/pkg/client/auth" | ||
| klog "k8s.io/klog/v2" | ||
| "k8s.io/utils/clock" | ||
| ctrl "sigs.k8s.io/controller-runtime" | ||
| "sigs.k8s.io/controller-runtime/pkg/client" | ||
|
|
||
| configv1 "github.com/openshift/api/config/v1" | ||
| "github.com/openshift/api/features" | ||
| configv1client "github.com/openshift/client-go/config/clientset/versioned" | ||
| configinformers "github.com/openshift/client-go/config/informers/externalversions" | ||
| "github.com/openshift/library-go/pkg/operator/configobserver/featuregates" | ||
| "github.com/openshift/library-go/pkg/operator/events" | ||
|
|
||
| "github.com/openshift/cluster-cloud-controller-manager-operator/pkg/controllers" | ||
| ) | ||
|
|
||
| var ( | ||
| scheme = runtime.NewScheme() | ||
| setupLog = ctrl.Log.WithName("setup") | ||
| ) | ||
|
|
||
| const ( | ||
| recorderName = "cloud-controller-manager-operator-node-label-sync-job" | ||
| missingVersion = "0.0.1-snapshot" | ||
| managedNamespace = controllers.DefaultManagedNamespace | ||
| featureGateWaitTimeout = 1 * time.Minute | ||
| // jobTimeout bounds the process's overall runtime safely below the Job manifest's | ||
| // activeDeadlineSeconds (300s), so it can log a clear error and exit cleanly instead of | ||
| // being killed by the kubelet once the deadline is hit. | ||
| jobTimeout = 4 * time.Minute | ||
| ) | ||
|
|
||
| func init() { | ||
| utilruntime.Must(clientgoscheme.AddToScheme(scheme)) | ||
| utilruntime.Must(configv1.AddToScheme(scheme)) | ||
| } | ||
|
|
||
| func main() { | ||
| klog.InitFlags(flag.CommandLine) | ||
| flag.Parse() | ||
|
|
||
| ctrl.SetLogger(klog.NewKlogr().WithName("VSphereNodeLabelSyncJob")) | ||
|
|
||
| ctx, cancel := context.WithTimeout(ctrl.SetupSignalHandler(), jobTimeout) | ||
| defer cancel() | ||
|
|
||
| restConfig := ctrl.GetConfigOrDie() | ||
|
|
||
| k8sClient, err := client.New(restConfig, client.Options{Scheme: scheme}) | ||
| if err != nil { | ||
| setupLog.Error(err, "unable to create client") | ||
| os.Exit(1) | ||
| } | ||
|
|
||
| configClient, err := configv1client.NewForConfig(restConfig) | ||
| if err != nil { | ||
| setupLog.Error(err, "unable to create config client") | ||
| os.Exit(1) | ||
| } | ||
|
|
||
| kubeClient, err := kubernetes.NewForConfig(restConfig) | ||
| if err != nil { | ||
| setupLog.Error(err, "unable to create kube client") | ||
| os.Exit(1) | ||
| } | ||
|
|
||
| configInformers := configinformers.NewSharedInformerFactory(configClient, 10*time.Minute) | ||
| controllerRef, err := events.GetControllerReferenceForCurrentPod(ctx, kubeClient, managedNamespace, nil) | ||
| if err != nil { | ||
| klog.Warningf("unable to get owner reference (falling back to namespace): %v", err) | ||
| } | ||
|
|
||
| featureGateAccessor := featuregates.NewFeatureGateAccess( | ||
| controllers.GetReleaseVersion(), missingVersion, | ||
| configInformers.Config().V1().ClusterVersions(), configInformers.Config().V1().FeatureGates(), | ||
| events.NewKubeRecorder(kubeClient.CoreV1().Events(managedNamespace), recorderName, controllerRef, clock.RealClock{}), | ||
| ) | ||
| featureGateAccessor.SetChangeHandler(func(featuregates.FeatureChange) {}) | ||
| go featureGateAccessor.Run(ctx) | ||
| go configInformers.Start(ctx.Done()) | ||
|
|
||
| select { | ||
| case <-featureGateAccessor.InitialFeatureGatesObserved(): | ||
| case <-ctx.Done(): | ||
| setupLog.Error(ctx.Err(), "context canceled while waiting for FeatureGate detection") | ||
| os.Exit(1) | ||
| case <-time.After(featureGateWaitTimeout): | ||
| setupLog.Error(errors.New("timed out waiting for FeatureGate detection"), "unable to determine feature gate state") | ||
| os.Exit(1) | ||
| } | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
|
|
||
| currentFeatureGates, err := featureGateAccessor.CurrentFeatureGates() | ||
| if err != nil { | ||
| setupLog.Error(err, "failed to get current feature gates") | ||
| os.Exit(1) | ||
| } | ||
| if !currentFeatureGates.Enabled(features.FeatureGateVSphereMixedNodeEnv) { | ||
| setupLog.Info("VSphereMixedNodeEnv feature gate is disabled, nothing to do") | ||
| return | ||
| } | ||
|
|
||
| if err := controllers.SyncVSphereNodeLabels(ctx, k8sClient); err != nil { | ||
| setupLog.Error(err, "failed to sync vSphere node labels") | ||
| os.Exit(1) | ||
| } | ||
|
|
||
| setupLog.Info("vSphere node label sync completed successfully") | ||
| } | ||
|
Comment on lines
+104
to
+145
Member
There was a problem hiding this comment. The job creates three K8s clients and a full informer framework to check one feature gate before doing a straightforward list+patch and for a run-once Job that feels a bit heavy. So I looked at whether we could replace the informer with a That said, you could still simplify things by moving the feature gate check into main.go.
Contributor
Author
There was a problem hiding this comment. ok, i'll look into this. |
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,112 @@ | ||
| 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 | ||
|
coderabbitai[bot] marked this conversation as resolved.
coderabbitai[bot] marked this conversation as resolved.
Comment on lines
+37
to
+44
Member
There was a problem hiding this comment. Could we make it run strictly only after CNI/OVN is up so we drop the hostNet?
Contributor
Author
There was a problem hiding this comment. Well not that it is run much later in the deployment, I can look into adjust this. |
||
| 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 | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| # 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 | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| volumes: | ||
| - name: host-etc-kube | ||
| hostPath: | ||
| path: /etc/kubernetes | ||
| type: Directory | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,80 @@ | ||
| package controllers | ||
|
|
||
| import ( | ||
| "context" | ||
| "errors" | ||
| "fmt" | ||
| "strings" | ||
|
|
||
| configv1 "github.com/openshift/api/config/v1" | ||
| corev1 "k8s.io/api/core/v1" | ||
| apierrors "k8s.io/apimachinery/pkg/api/errors" | ||
| klog "k8s.io/klog/v2" | ||
| "sigs.k8s.io/controller-runtime/pkg/client" | ||
|
|
||
| "github.com/openshift/cluster-cloud-controller-manager-operator/pkg/cloud/vsphere" | ||
| ) | ||
|
|
||
| // SyncVSphereNodeLabels performs a single pass over all nodes and retroactively applies the | ||
| // node.openshift.io/platform-type=vsphere label to vSphere nodes that are missing it. The vSphere | ||
| // CCM only ever applies this label (via its --node-labels flag, see pkg/cloud/vsphere) when it | ||
| // initializes a node for the first time, so nodes that joined the cluster before the | ||
| // VSphereMixedNodeEnv feature gate was enabled never get labeled without this repair pass. Hybrid | ||
| // 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. | ||
| func SyncVSphereNodeLabels(ctx context.Context, c client.Client) error { | ||
| infra := &configv1.Infrastructure{} | ||
| if err := c.Get(ctx, client.ObjectKey{Name: infrastructureResourceName}, infra); err != nil { | ||
| if apierrors.IsNotFound(err) { | ||
| klog.Infof("infrastructure resource not found, skipping node label sync") | ||
| return nil | ||
| } | ||
| return 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") | ||
| return nil | ||
| } | ||
|
|
||
| nodeList := &corev1.NodeList{} | ||
| if err := c.List(ctx, nodeList); err != nil { | ||
| return fmt.Errorf("failed to list nodes: %w", err) | ||
| } | ||
|
|
||
| var errs []error | ||
| for i := range nodeList.Items { | ||
| if err := syncVSphereNodeLabel(ctx, c, &nodeList.Items[i]); err != nil { | ||
| errs = append(errs, fmt.Errorf("node %s: %w", nodeList.Items[i].Name, err)) | ||
| } | ||
| } | ||
|
|
||
| return errors.Join(errs...) | ||
| } | ||
|
|
||
| func syncVSphereNodeLabel(ctx context.Context, c client.Client, node *corev1.Node) error { | ||
| if !strings.HasPrefix(node.Spec.ProviderID, vsphere.NodeProviderIDPrefix) { | ||
| return nil | ||
| } | ||
|
|
||
| if _, ok := node.Labels[vsphere.NodePlatformTypeLabelKey]; ok { | ||
| return nil | ||
| } | ||
|
|
||
| patch := client.MergeFrom(node.DeepCopy()) | ||
| if node.Labels == nil { | ||
| node.Labels = map[string]string{} | ||
| } | ||
| node.Labels[vsphere.NodePlatformTypeLabelKey] = vsphere.NodePlatformTypeLabelValueVSphere | ||
|
|
||
| if err := c.Patch(ctx, node, patch); err != nil { | ||
| return fmt.Errorf("failed to patch label: %w", err) | ||
| } | ||
|
|
||
| klog.Infof("added %s=%s label to node %s", vsphere.NodePlatformTypeLabelKey, vsphere.NodePlatformTypeLabelValueVSphere, node.Name) | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| return nil | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are not registering these for the fake client in the tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could do this in a TestMain or at the top of the test, matching what the real main.go does in its init()