Skip to content
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

✨ Add an IgnoreFields option to select whether to override resource by mw #345

Conversation

qiujian16
Copy link
Member

@qiujian16 qiujian16 commented Nov 12, 2024

The option is to disable override resource on the spoke by manifestwork when the resource is changed by another actor on the spoke

Summary

Related issue(s)

Fixes #

@openshift-ci openshift-ci bot requested review from deads2k and mdelder November 12, 2024 09:35
Copy link
Contributor

openshift-ci bot commented Nov 12, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: qiujian16

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

The option is to disable overrid resource on the spoke by manifestwork
when the resource is changed by another actor on the spoke

Signed-off-by: Jian Qiu <[email protected]>
@qiujian16 qiujian16 force-pushed the mw-update-strategy branch 3 times, most recently from 3d53e1d to dc501bb Compare November 26, 2024 02:26
@qiujian16 qiujian16 changed the title ✨ Add an option to select override resource by mw ✨ Add an IgnoreFields option to select whether to override resource by mw Nov 26, 2024
Condition IgnoreFieldsCondition `json:"condition"`

// JSONPaths defines the list of json path in the resource to be ignored
JSONPaths []string `json:"jsonPaths"`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this be a required field? if it is empty, what should we do?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not think list can be required, but we could set the minItem to 1

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated

@zhujian7
Copy link
Member

LGTM
/assign @elgnay @haoqing0110

@elgnay
Copy link
Contributor

elgnay commented Nov 26, 2024

/lgtm

@openshift-ci openshift-ci bot added the lgtm label Nov 26, 2024
@openshift-merge-bot openshift-merge-bot bot merged commit 05ff7c1 into open-cluster-management-io:main Nov 26, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants