-
Notifications
You must be signed in to change notification settings - Fork 40
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
✨ Support setting manifest work config for agents #214
✨ Support setting manifest work config for agents #214
Conversation
|
||
func containsFeedbackRule(rules []workapiv1.FeedbackRule, rule workapiv1.FeedbackRule) bool { | ||
for _, r := range rules { | ||
if reflect.DeepEqual(r, rule) { |
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.
actually we may only compare the name of the rule. It won't work if you have two rules with the same name.
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.
you mean the jsonpath.name? so if they have the same name but a different path, should we override the old one with the agentOption.ManifestConfigs?
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.
Yes, we have to decide override or not, but comparing name only should be good enough.
Signed-off-by: zhujian <jiazhu@redhat.com>
479494c
to
16f8c4f
Compare
Signed-off-by: zhujian <jiazhu@redhat.com>
@@ -518,18 +517,17 @@ func getManifestConfigOption(agentAddon agent.AgentAddon, | |||
} | |||
} | |||
|
|||
emc := agentAddon.GetAgentAddonOptions().ManifestConfigs | |||
for _, mc := range emc { | |||
aaomc := agentAddon.GetAgentAddonOptions().ManifestConfigs |
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.
what does aaomc stands for :). You might want a more meaningful name, event it is longer.
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.
agentAddonOptionsManifestConfigs, but I changed it to userConfiguredManifestConfigs
Signed-off-by: zhujian <jiazhu@redhat.com>
d18f5e9
to
155c9e5
Compare
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: qiujian16, zhujian7 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 |
ec48c5d
into
open-cluster-management-io:main
With this, the addonTemplate can set the
ManifestConfigs
for the generated manifestworks