-
Notifications
You must be signed in to change notification settings - Fork 226
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 flag for status policy #4129
Conversation
884a708
to
d9df85a
Compare
d9df85a
to
07dddaa
Compare
Rebased on main; the failing checks seem unrelated, and go generate works locally after rebase. |
Failure looks unrelated; let me know if I should rebase again. |
/retest |
Thank you @fsommar - there's some code repetition here in the existing code (it looks like we started live.NewClusterClientFactory but never really finished it). I'm OK to merge as-is, but I also am checking whether I have permissions to merge kubernetes-sigs/cli-utils#637 |
Looks like I don't have permissions, so I'm going to apply hold & approve & lgtm. If you want to merge before the cli-utils PR, just remove the hold :-) /approve |
d0a73be
to
5970aa9
Compare
I've upgrade to the latest cli-utils release with my changes and cleaned up the history. /assign justinsb |
/hold cancel |
The PR that removes Porch from the repo has merged. Can you rebase this one and it should address most of the test failures? |
Signed-off-by: Fredrik Sommar <fred.sommar+github@gmail.com>
The fix kyaml function is no longer available in the same location upstream, and because there are no other dependencies here I decided to remove it. Signed-off-by: Fredrik Sommar <fred.sommar+github@gmail.com>
The default behavior is still to use --status-policy=all. For users that want to avoid the status behavior for various reasons. Signed-off-by: Fredrik Sommar <fred.sommar+github@gmail.com>
5970aa9
to
e464356
Compare
It went through without tests running 😅 At least they were passing locally. I noticed too late that I accidentally included the go version files in the porch directory after rebasing |
The default behavior is still to use --status-policy=all. The option is there for users that want to avoid the status behavior for various reasons. In our case, we use
kpt live apply
in our CI/CD pipelines, but don't need it saving statuses for all resources into the inventory. Thus, we want to be able to pass--status-policy=none
.I created a partner PR in kubernetes-sigs/cli-utils#637. The idea is to have that merged first and then rely on that flagutils func in this PR.
If this flag has been intentional withheld from users, can we consider having this flag hidden instead?