-
Notifications
You must be signed in to change notification settings - Fork 21
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
provide option to configure agent install strategy #10
provide option to configure agent install strategy #10
Conversation
pkg/addon/manager/addon.go
Outdated
@@ -52,7 +52,7 @@ func (m *managedServiceAccountAddonAgent) GetAgentAddonOptions() agent.AgentAddo | |||
CSRApproveCheck: agent.ApprovalAllCSRs, | |||
PermissionConfig: m.setupPermission, | |||
}, | |||
InstallStrategy: agent.InstallAllStrategy(common.AddonAgentInstallNamespace), | |||
InstallStrategy: nil, |
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 should introduce a flag for this
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.
i think so, let's add an flag
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.
Good plan, I will add a flag
What should be the default behavior?
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.
Let's set the default behavior as-is
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.
updated the default value in the chart
318d4fc
to
8843eff
Compare
/retest |
64678bc
to
9eacb27
Compare
charts/managed-serviceaccount/templates/manager-deployment.yaml
Outdated
Show resolved
Hide resolved
4f962a0
to
4434102
Compare
Signed-off-by: Hao Liu <haoli@redhat.com>
4434102
to
2ca9adb
Compare
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: qiujian16, TheRealHaoLiu 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 |
…/cherry-pick-mar-10 Cherry pick mar 10
previous strategy enable managed-serviceaccount addon on all managedclusters, user is not able to disable the addon by removing the ManagedClusterAddon resource
the proposed change add a new option
agent-install-all
to the binaryif
agent-install-all == true
enable agent on all managed clustersif
agent-install-all == false
do not enable agent by default and the addon will only be enabled when a ManagedClusterAddon is created in the cluster namespace and will be disabled when the ManagedClusterAddon is deletedexample ManagedClusterAddon