Skip to content

Commit bc26dbe

Browse files
committed
control-plane-operator/controllers/hostedcontrolplane/v2/cvo: Consume include.release.openshift.io/bootstrap-cluster-version-operator annotation
The cluster-version operator has a complicated system for deciding whether a given release-image manifest should be managed in the current cluster [1,2]. Implementing that system here, or even using library-go and remembering to vendor-bump here, both seem like an annoying maintenance load. We could use the CVO's render command like the standalone installer [3,4], but that logic is fairly complicated because it needs to generate all the artifacts necessary for bootstrap MachineConfig rendering, or the production machine-config operator will complain about MachineConfigPools requesting rendered-... MachineConfig that don't exist. All we actually need out of the bootstrap container are the resources that the cluster-version operator needs to launch and run, which are labeled with the grep target since [5]. That avoids installing anything the cluster doesn't actually need here by mistake. Once the production CVO container starts, it will apply the remaining resources that the cluster actually needs. The new "is there a .status.history entry?" guard keeps this loop from running if we already have a functioning cluster-version operator (we don't want to be wrestling with the CVO over the state of the ClusterVersion CRD). The 'oc apply' (instead of 'oc create') gives us a clear "all of those exist now" exit code we can use to break out of the loop during the initial setup (because this init-container needs to complete before the long-running CVO container can start). I'm also dropping the openshift-config and openshift-config-managed Namespace creation. They are from a30db71 (Refactor cluster-version-operator, 2024-11-18, openshift#5125), but that commit doesn't explain why they were added or hint at where they lived before (if anywhere). I would expect the cluster-version operator to be able to create those Namespaces from the release-image manifests when they are needed, as with other cluster resources. I'm also shifting the ClusterVersion custom resource apply into the loop, to avoid attempting to apply before the ClusterVersion CRD exists and to more gracefully recover from temporary API hiccup sorts of things. I'm also adding some debugging echos and other output to make it easier to debug "hey, why is it applying these resources that I didn't expect it to?" or "... not applying the resources I did expect?". [1]: https://github.com/openshift/enhancements/blob/2b38513b8661632f08e64f4acc3b856e842f8669/dev-guide/cluster-version-operator/dev/operators.md#manifest-inclusion-annotations [2]: https://github.com/openshift/library-go/blob/ac826d10cb4081fe3034b027863c08953d95f602/pkg/manifest/manifest.go#L296-L376 [3]: https://github.com/openshift/installer/blob/a300d8c0e9d9d566a85740244a7da74d3d63e23c/data/data/bootstrap/files/usr/local/bin/bootkube.sh.template#L189-L216 [4]: https://github.com/openshift/cluster-version-operator/blob/eaf28f5165bde27435b0f0c9a69458677034a58d/pkg/payload/render.go [5]: openshift/cluster-version-operator#1352
1 parent 64e96a9 commit bc26dbe

2 files changed

Lines changed: 4 additions & 18 deletions

File tree

control-plane-operator/controllers/hostedcontrolplane/v2/assets/cluster-version-operator/deployment.yaml

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -91,13 +91,11 @@ spec:
9191
cat > /tmp/clusterversion.json <<EOF
9292
${CLUSTER_VERSION_JSON}
9393
EOF
94-
oc get ns openshift-config &> /dev/null || oc create ns openshift-config
95-
oc get ns openshift-config-managed &> /dev/null || oc create ns openshift-config-managed
96-
oc apply -f /var/payload/manifests/0000_00_cluster-version-operator_01_clusterversions*
97-
oc apply -f /tmp/clusterversion.json
98-
while true; do
94+
echo 'Deciding whether bootstrap resources need to be applied...'
95+
ls -l $(grep -rl 'include.release.openshift.io/bootstrap-cluster-version-operator: .*hypershift' /var/payload/manifests) /tmp/clusterversion.json
96+
while test -z "$(oc get -o jsonpath='{.status.history[0].version}' clusterversions.config.openshift.io version)"; do
9997
echo "Applying CVO bootstrap manifests..."
100-
if oc apply -f /var/payload/manifests; then
98+
if oc apply $(grep -rl 'include.release.openshift.io/bootstrap-cluster-version-operator: .*hypershift' /var/payload/manifests | sed 's/^/-f /') -f /tmp/clusterversion.json; then
10199
echo "Bootstrap manifests applied successfully."
102100
break
103101
fi

control-plane-operator/controllers/hostedcontrolplane/v2/cvo/deployment.go

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -204,18 +204,6 @@ func preparePayloadScript(platformType hyperv1.PlatformType, oauthEnabled bool,
204204
fmt.Sprintf("cp -R /release-manifests %s/", payloadDir),
205205
)
206206

207-
// NOTE: We would need to leverage part of the manifest.Include logic (https://github.com/openshift/library-go/blob/0064ad7bd060b9fd52f7840972c1d3e72186d0f0/pkg/manifest/manifest.go#L190-L196)
208-
// to properly evaluate which CVO manifests to select based on featureset.
209-
// We only have access to bash, so we must filter based on the feature-set annotation in the manifests manually.
210-
// Any file that has a feature-set annotation must have a value that matches the current feature set else it is removed.
211-
// Files that do not have a feature-set annotation are included unconditionally.
212-
stmts = append(stmts, fmt.Sprintf(`for file in $(find %s/manifests/ -name "*.yaml"); do
213-
IFS=',' read -ra feature_sets <<< "$(cat $file | grep "release.openshift.io/feature-set:" | awk '{print $2}')"
214-
if [[ "${#feature_sets[@]}" -gt 0 ]] && ! [[ " ${feature_sets[*]} " =~ "%s" ]]; then
215-
rm -vf $file
216-
fi
217-
done`, payloadDir, featureSet))
218-
219207
for _, manifest := range manifestsToOmit {
220208
if platformType == hyperv1.IBMCloudPlatform || platformType == hyperv1.PowerVSPlatform {
221209
if manifest == "0000_50_cluster-storage-operator_10_deployment-ibm-cloud-managed.yaml" || manifest == "0000_50_cluster-csi-snapshot-controller-operator_07_deployment-ibm-cloud-managed.yaml" {

0 commit comments

Comments
 (0)