-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Conversation
Signed-off-by: ahached <hachedahmeddev@gmail.com>
I think we can merge this for 1.23.0 I do think we may have to bump the minor version, if it is a change that adds functionality. |
There might be a great reason, but for my info it would be nice to ask before merging an update: Why not update all the way to Kustomize 3.9.3 (or I guess 3.10.0, which kustomize-controller is now on?) That is what version Flux v2 is currently stuck at, due to I think the removal of some remote base related support in Kustomize 4 and go-getter. The real big question is, I guess, what will be done to reconcile the development here? Kustomize is on 4.0.5 now, the release notes for Kustomize 4 explain why we can't upgrade without a breaking change and fluxcd/flux2#918 is open regarding that question. |
In fact I chose the kustomize 3.8.5 because it's the least version that contains the var ref in deployment template annotations. But, if we are able to upgrade the kustomize to a more recent version, it will be a great idea. In my company we are using fluxv1, and sadly moving to the fluxv2 is not yet planned, we prefer to stick to the fluxv1 for the time being. |
I think the maintainership of Flux will prefer to be on just one version of Kustomize, the latest. Given that Kustomize 4 has made this choice, it can't be the latest. But in general when I am making decisions about how to proceed with changes that might technically be considered functional / feature updates, I am aiming to move Flux v1 toward closer convergence with Flux v2. This would suggest we should upgrade to Kustomize 3.10.0, so long as there aren't any behavior changes that could have been considered functional regressions (and based on the SIG CLI meetings I've been to, and what sometimes seems like an excessive bit of formality around the topic of backwards compatibility, I think we can say confidently there won't be any such regressions, at least none that are placed intentionally without a corresponding MAJOR version bump.) I will bring this topic up at the Flux weekly meeting on Thursday if we haven't made a decision before then. I'm leaning towards recommending that we upgrade to 3.10.0, and stay in sync with Kustomize Controller through the remainder of the maintenance period, how ever long that should be for Flux v1. 👍 |
Do you want to update your PR to point at 3.10.0? We may not do another release until about two weeks. I would like to get back into a cadence of releases every 30 days, if there are changes in the queue that can be merged and released. Patch releases that are small or urgent can be more frequent in between, but I think it will be better for the maintenance cycles of the project if we do not go too long without at least bumping dependency versions. Once a month seems like a good rhythm to maintain within the context of maintenance mode. (I do not know if you've already updated your local release, or if you are waiting for this PR to merge and go into a release...) |
Signed-off-by: ahached <hachedahmeddev@gmail.com>
1fe9749
to
0a3fad7
Compare
Signed-off-by: ahached <hachedahmeddev@gmail.com>
d61ab88
to
c838a9f
Compare
@kingdonb, I updated the PR to point at kustomize v3.10.0. |
Because it is a MINOR dependency version bump, we will also want to increment the minor version of Flux. I will take care to rebase and merge this when the time comes for a next release. (I think 1.23.0 will be our next release.) |
Just aiming to understand and somewhat describe a bit locally here what behavior is being enabled, at least enough so I can explain it if needed. I did some reading on varReference clicking through the link that you provided. 👍 So, if I understand correctly, this is not a new feature, it's just a new default in Kustomize. Before this update, you had to update varReference with your own If I am understanding correctly, also, this feature has fallen out of favor in Kustomize. kubernetes-sigs/kustomize#2052 From reading this, they would prefer you use envsubst, which I guess doesn't restrict the locations that you can target with a variable for replacement. FYI, a recent release of Flux v1 added envsubst as a built-in binary for Kustomize build to use (#3138), and Flux v2 supports envsubst natively as well. Funny enough, my understanding is the Flux maintainers also don't like this feature very much (envsubst)... This will still merge, nothing has changed about that; I was just looking to explore and provide the background for anyone else that isn't aware of the details. |
So, to reiterate the somewhat poorly thought-out position I stated earlier, Flux v2 is on Kustomize 3.10.0 and the next release plans to upgrade Flux v1 to match that minor upgrade, in the next minor series (Flux v1.23.0) I stated that Flux v1 would try to keep in sync with Flux v2 through the duration of maintenance, but that may not be possible. There is fluxcd/kustomize-controller#343 open now, which plans to upgrade Flux v2 to Kustomize v4, and take the breaking changes that come with that transition. That was inevitable and I should have expected would happen. Flux v1 cannot follow to Kustomize v4, as that will represent a breaking change (and as we've held consistently, no breaking changes can be landed in Flux v1 during the scope of changes we defined as "maintenance mode.") |
@kingdonb fluxv1 user here, that uses the vars pattern extensively referenced above and who has just recently bumped their flux instance to use kustomize 4.1.2. No problems to report. Can you clarify what you think has been "broken" ? Happy to talk on Slack, and summarise here if you want to back and forth a bit. |
If the consensus is there is a way to move to Kustomize v4 into Flux v1 without breaking anything for existing Flux users, I'll be happy to go with the consensus of the maintainers and make the update. But the kustomize CLI has bumped their major version, which is meant to indicate that there have been breaking changes according to semver. In Flux v1.22.2, a patch release, we updated Kustomize to the latest release within the same MINOR number v3.8. I'm following the same logic that says feature updates or breaking changes in a dependency require a MINOR or MAJOR version bump, respectively. If the truth is something different, we can potentially make different decisions in context as long as the reasoning is documented, but in the maintenance period we've committed to follow the semver spec and not break anything in Flux v1. Let's start a separate discussion if you need Kustomize v4 since right now, there is nothing holding back Kustomize v3.10.0 from landing in the next Flux v1 MINOR release, but we could for example find out there have been breaking changes in there and it's not suitable for including in the next release. So far I haven't really seen enough evidence from my own limited uses to make a judgement about it! I want to stay in sync with Flux v2 as much as possible. I do not want to add maintenance burden to existing Flux v1 users who are maybe not able to allocate nominal resources even for the migration effort to Flux v2, (not to belabor the point, but v1 should be stable.) My impression is based solely on the fluxcd/flux2#918 pinned issue titled "Kustomize v4 High Impact Breaking Changes" and I do not know specifically what changes in Kustomize v4 are breaking. I believe they are related to git remote bases and the removal of go-getter. It is also possible the breaking parts of Kustomize v4 have been remediated in recent Kustomize releases, (I haven't really followed the development very closely over there.) |
Update kustomize to v3.10.0 to leverage the var ref in deployment template annotations.
https://github.com/kubernetes-sigs/kustomize/blob/kustomize/v3.10.0/api/konfig/builtinpluginconsts/varreference.go
Closes #3457