Introduce kustomize build options #303
Conversation
There was a problem hiding this comment.
As usual, thanks for the PR @ron1! It LGTM at a first pass, barring a few comments.
|
@ron1 Thanks for your PR! I am down for upgrading to latest kustomize v4, but do you know any specific reason that v4 is going to create an incompatible issue against v3? |
|
@StrongMonkey See the Kustomize v4 and v3.10.0 release notes here. My biggest concerns are:
Since Fleet is relatively new, maybe it can get away with some backwards incompatibility. Also, Kustomize 4 now re-syncs with Let me know your thoughts. |
|
@StrongMonkey @nickgerace Have you decided on a path forward regarding this patch? It would be great to get this patch into a 0.3.5 release candidate if possible. |
|
@ron1 May you rebase first? I want to see if the CI runs pass first. I think this approach is okay with me, but would love @StrongMonkey's take. I'll re-review after rebase |
ron1
force-pushed
the
introduce-kustomize-buildOptions
branch
from
e604342 to
7a7402f
Compare
|
@nickgerace I pushed the rebased branch. I'll look forward to your re-review and feedback. Thanks! |
|
@nickgerace Working on it now. |
ron1
force-pushed
the
introduce-kustomize-buildOptions
branch
from
7a7402f to
fb7c563
Compare
|
@nickgerace Just pushed a rebase on your kustomize v3.8.10 upgrade commit. Remember that my commit depends upon this kustomize fork which would presumably need to be moved to repo rancher/kustomize. |
There was a problem hiding this comment.
Given our previous conversations and PRs/issues, this LGTM, but I would like an approval from @StrongMonkey and/or @ibuildthecloud first. As usual, thanks @ron1
|
Hi @StrongMonkey. Would you have some time to review this small PR? I was looking forward to this feature landing in Rancher 2.5.8. |
|
@ron1 This is the commit you added in your fork? ron1/kustomize@affd5ec |
@StrongMonkey Correct! The fork is inspired by the refactored code in kustomize v4. So, once fleet upgrades to kustomize v4, the Rancher kustomize fork will no longer be needed. Note that most of the code in the fork is a unit test to validate the new functionality. |
ron1
force-pushed
the
introduce-kustomize-buildOptions
branch
from
fb7c563 to
c9ae9c3
Compare
|
@nickgerace @StrongMonkey Now that fleet has upgraded to kustomize 4.1.2, I force pushed a revised PR for this feature that no longer has a dependency on a forked kustomize v3 tag. Please review and provide feedback. Thanks, Ron |
ron1
force-pushed
the
introduce-kustomize-buildOptions
branch
from
c9ae9c3 to
ed750b1
Compare
|
I think this is a pretty low risk commit. Any way this could be included in Rancher 2.6.3? |
ron1
force-pushed
the
introduce-kustomize-buildOptions
branch
from
ed750b1 to
8d30b85
Compare
|
@kinarashah Are you able to review this PR? It has been open for a while now. |
kinarashah
requested review from
aiyengar2
and removed request for
StrongMonkey and
kinarashah
|
@ron1 Thanks for the reminder, I am not aware of changes in fleet recently. Pinging @aiyengar2 and @prachidamle for review. |
|
Hi, |
|
This seems outdated. |
This PR resolves #155 and has a dependency on forked kustomize v3 tag https://github.com/ron1/kustomize/releases/tag/kustomize%2Fv3.8.10-fleet4. Once Fleet and the Kustomize community move to kustomize v4, the dependency on a forked kustomize repo will be no longer since the build cmd in kustomize v4 exposes the apis needed by Fleet.