-
Notifications
You must be signed in to change notification settings - Fork 35
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
CM-424: Allow feature specific controller gating for operator TechPreview features #229
base: master
Are you sure you want to change the base?
Conversation
@swghosh: This pull request references CM-424 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target either version "4.19." or "openshift-4.19.", but it targets "cert-manager-1.15" instead. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: swghosh The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
d47b9ea
to
2af5796
Compare
@swghosh: This pull request references CM-424 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target either version "4.19." or "openshift-4.19.", but it targets "cert-manager-1.15" instead. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
@swghosh: This pull request references CM-424 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target either version "4.19." or "openshift-4.19.", but it targets "cert-manager-1.15" instead. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
a2306c2
to
cbacb19
Compare
@swghosh: This pull request references CM-424 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target either version "4.19." or "openshift-4.19.", but it targets "cert-manager-1.15" instead. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
// For more details, check Reconcile and its Result here: | ||
// - https://pkg.go.dev/sigs.k8s.io/[email protected]/pkg/reconcile | ||
// Reconcile is run each time the CertManager object has a create/update/delete event. | ||
// All objects other than certmanager.operator.openshift.io/cluster lead to no-op syncs. | ||
func (r *CertManagerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { |
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.
nit: We could add couple of UTs here I think.
fc.log.V(1).Info("graceful shutdown") | ||
return | ||
|
||
case <-time.Tick(retryInterval): |
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.
not a blocker, instead of periodic poll, could we share a channel between Reconcile
and RunWithManagerOnceEnabled
methods, say a new field in CertManagerReconciler
and as arg to the later method, and wait on the channel for updates?
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.
Added an event watcher to the FeatureAccessor, which takes care of doing the similar. Events are sent by the feature accessor when a new feature is enabled, this is subscribed by the RunWithManagerOnceEnabled
method which triggers enablement accordingly.
to pkg/controller/certmanager Signed-off-by: Swarup Ghosh <[email protected]>
Signed-off-by: Swarup Ghosh <[email protected]>
allows setting OLM operator condition Signed-off-by: Swarup Ghosh <[email protected]>
8f80c5e
to
2f28502
Compare
Updated the implementation for the FeatureAccessor to wrap with an |
- reconcile certmanager.operator CR with ctrl runtime (feature gates enable check) - add feature_controller_set with ManagedController(s) Signed-off-by: Swarup Ghosh <[email protected]>
/test verify-deps |
allows using envtest tools for CRD validation tests Signed-off-by: Swarup Ghosh <[email protected]>
through envtest based CRD validation Signed-off-by: Swarup Ghosh <[email protected]>
@swghosh: all tests passed! Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
dynamically starting controllers after controller-runtime has started: kubernetes-sigs/controller-runtime#1994 (comment).