Repository navigation
Conversation
Allow registry+v1 bundles to ship a DaemonSet in manifests/, for operators that need an agent on every node, such as a CSI node plugin. The gate is alpha and disabled by default, and it has no effect unless BoxcutterRuntime is also enabled, since that runtime provides the DaemonSet progression probe. Validators take no render options, so CheckObjectSupport now leaves DaemonSet to the BundleAdditionalResourcesGenerator, which accepts it only when the renderer runs with WithDaemonSetSupport. With the gate disabled, a bundle with a DaemonSet is still rejected, now at render time. With it enabled, the DaemonSet is rendered into the install namespace. Signed-off-by: Cesar Celis <celis.hernandez.cesar@gmail.com>
Treat a DaemonSet as rolled out when status.updatedNumberScheduled and status.numberAvailable both equal status.desiredNumberScheduled. The object-controller already wraps every progression probe in an observed generation check, so a stale status reports Unknown. Signed-off-by: Cesar Celis <celis.hernandez.cesar@gmail.com>
Cover the three cases of the RegistryV1DaemonSetSupport gate: a bundle with a DaemonSet installs and rolls out, a DaemonSet that never becomes available exceeds the progress deadline, and the bundle is rejected when the gate is disabled. Scenarios tagged @<FeatureGate>=false now run only when that feature gate is disabled. Signed-off-by: Cesar Celis <celis.hernandez.cesar@gmail.com>
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: true
Comment |
✅ Deploy Preview for olmv1 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Author
|
Closing in favor of the OLMv1 Helm support path described in #3056 (comment). |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Description
Operators that need an agent on every node, such as CSI drivers with a node plugin, cannot ship that DaemonSet in a registry+v1 bundle today, because the renderer rejects it in
manifests/, so they create it at runtime from their controller. This adds an alphaRegistryV1DaemonSetSupportfeature gate, disabled by default. With the gate enabled, a DaemonSet in the bundle is rendered into the install namespace and takes part in rollout progression through a new DaemonSet progression probe (updatedNumberScheduledandnumberAvailableequal todesiredNumberScheduled, inside the existing observed generation check). The gate has no effect unless BoxcutterRuntime is also enabled.spec.config(nodeSelector, tolerations, env) is applied to the CSV's Deployments only, not to this DaemonSet. With the gate disabled the bundle is still rejected, now at render time byBundleAdditionalResourcesGenerator(bundle contains unsupported resource: Name: <name>, Kind: DaemonSet) instead of by the validator. Validators take no render options, soCheckObjectSupportnow leaves DaemonSet toBundleAdditionalResourcesGenerator, which enforces the gate; the alternative is a separate validator list for the gated renderer, which I am happy to switch to. One known limit, shared with the Deployment probe: a DaemonSet that schedules zero pods never passes, because the API server omits the zero-valuedupdatedNumberScheduledandnumberAvailable; the Deployment probe behaves the same withreplicas: 0(status.updatedReplicasmissing). Unlikereplicas: 0, a DaemonSet can reach zero scheduled pods from cluster state (a nodeSelector that matches no node yet), so this is more likely to be hit; the same goes for a node that is not Ready, since every scheduled pod must be available. A CEL assertion type, which boxcutter already provides asprobing.NewCELProbe, would fix both, and I can follow up with it. Unit tests cover the validator, generator, renderer, provider gate, the BoxcutterRuntime requirement and probe; e2e covers install with the gate enabled, a DaemonSet that never becomes available hitting the progress deadline, and rejection with the gate disabled (via a new@<FeatureGate>=falsescenario tag). Refs #3056.Tested with
make lint,make test-unit,make verify, and the standard and experimental e2e suites on kind. All scenarios pass exceptproxy.feature:40, which fails the same way onmainin my environment (the proxy listens on the host, which my kind network does not reach). After the last change I re-ran the DaemonSet scenarios only.Reviewer Checklist