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

Update instructions for Kustomize 5 #2399

Merged

Conversation

kimwnasptd
Copy link
Member

Refs #2388

This PR updates the instructions and the example kustomization to work with Kustomize 5. Note that after this PR is merged users won't be able to use earlier versions of kustomize, since we need the sortOptions feature of Kustomize 5 #2388 (comment). This holds for the one-liner install. The manual instructions are not affected and people can use earlier versions of kustomize there.

Next work items, although not critical for the KF 1.7 release, are to update our manifests so that we don't see warnings from Kustomize #2388 (comment)

cc @DomFleischmann @annajung @juliusvonkohout @jbottum
/cc @kubeflow/wg-manifests-leads

Add the sortOptions in the kustomization.yaml to ensure we are creating
MutationWebhooks before Deployments and Pods. This way we'll ensure that
Pods will get Istio sidecars in a race-free way.

Signed-off-by: Kimonas Sotirchos <kimwnasptd@arrikto.com>
Update instructions to inform users to use Kustomize 5. We'll also need
to do an awk command, to remove a line that ends up in the final output.
That is a regression on Kustomize's side. Once a newer version of
Kustomize is available with the fix we'll then remove this awk command.

Signed-off-by: Kimonas Sotirchos <kimwnasptd@arrikto.com>
Update the helper script to use Kustomize 5 for the tests.

Signed-off-by: Kimonas Sotirchos <kimwnasptd@arrikto.com>
Update the GH Action that runs the unit test to use the same script we
use in the other GH Actions for fetching kustomize.

Signed-off-by: Kimonas Sotirchos <kimwnasptd@arrikto.com>
@google-oss-prow google-oss-prow bot requested a review from a team February 23, 2023 15:00
@google-oss-prow
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kimwnasptd

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

@annajung
Copy link
Member

Thanks Kimonas! I couldn't recreate the race condition described in the #1797 with kustomize 5, but can confirm that with sortOptions added, it does create MutatingWebhookConfiguration before the necessary resources.

Testing with kustomize build example, only difference between kustomize5 with sortOptions enabled and kustomize3 was the order of when PVCs were created. It looks like kustomize3 places PVC at the end right before the ValidatingWebhookConfiguration, so kustomize5 order is preferrable IMO

lgtm (not adding the actual label to allow other folks to review)

there is still ongoing discussion on if it should be included in 1.7 release in #2388 but should not be a blocker for this PR.

@@ -438,7 +440,3 @@ For security reasons, we don't want to use the default password for the default

- **Q:** What versions of Istio, Knative, Cert-Manager, Argo, ... are compatible with Kubeflow? \
**A:** Please refer to each individual component's documentation for a dependency compatibility range. For Istio, Knative, Dex, Cert-Manager and OIDC-AuthService, the versions in `common` are the ones we have validated.
Copy link
Contributor

@DomFleischmann DomFleischmann Mar 10, 2023

Choose a reason for hiding this comment

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

Here is a suggestion for the Kustomize 3.2 QA section.

Suggested change
**A:** Please refer to each individual component's documentation for a dependency compatibility range. For Istio, Knative, Dex, Cert-Manager and OIDC-AuthService, the versions in `common` are the ones we have validated.
**A:** Please refer to each individual component's documentation for a dependency compatibility range. For Istio, Knative, Dex, Cert-Manager and OIDC-AuthService, the versions in `common` are the ones we have validated.
- **Q: ** How can I still use Kustomize version 3.2?
**A: ** The only change incompatible with Kustomize 3.2 is the `sortOptions` section in the `example/kustomization.yaml` file. I you want to use Kustomize 3.2 just comment out the entire section (`#` character in front of each line) and execute the one-liner.

Copy link
Member Author

Choose a reason for hiding this comment

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

@DomFleischmann I pushed a commit with a slightly refactored section for the earlier version of Kustomize. PTAL

Signed-off-by: Kimonas Sotirchos <kimwnasptd@arrikto.com>
@kimwnasptd
Copy link
Member Author

Adding a summary here as well from the last community (Tue 28 Feb) and release meeting (Mon 6 March). We decided to move forward with using Kustomize 5 as the default installation tool.

The rationale is that:

  1. With feedback until now we have heard from distros that they managed to use Kustomize 5 without any issues
  2. There are no (currently known) deprecations that affect our use of kustomize
  3. We'd like to move to a newer version of Kustomize, which hasn't been updated for a long time
  4. We have documented a clear path on how people can use earlier versions of Kustomize, if they need to

So at this point we have confidence from both Manifests WG and feedback from distros that Kustomize 5 does not have any issues we haven't yet documented. Plus if adopters are not ready for the version switch they can still use an earlier version of kustomize. So let's move forward with this PR and also cherry-pick it to the KF 1.7 release branch

@DomFleischmann
Copy link
Contributor

/lgtm

@google-oss-prow google-oss-prow bot added the lgtm label Mar 13, 2023
@google-oss-prow google-oss-prow bot merged commit 74eb8d9 into kubeflow:master Mar 13, 2023
@kimwnasptd kimwnasptd deleted the feature-kimwnasptd-kustomize-5 branch March 13, 2023 15:21
DomFleischmann pushed a commit to DomFleischmann/manifests that referenced this pull request Mar 13, 2023
* example: Update for kustomize 5

Add the sortOptions in the kustomization.yaml to ensure we are creating
MutationWebhooks before Deployments and Pods. This way we'll ensure that
Pods will get Istio sidecars in a race-free way.

Signed-off-by: Kimonas Sotirchos <kimwnasptd@arrikto.com>

* readme: Update instructions to use Kustomize 5

Update instructions to inform users to use Kustomize 5. We'll also need
to do an awk command, to remove a line that ends up in the final output.
That is a regression on Kustomize's side. Once a newer version of
Kustomize is available with the fix we'll then remove this awk command.

Signed-off-by: Kimonas Sotirchos <kimwnasptd@arrikto.com>

* gh-action: Update to Kustomize 5

Update the helper script to use Kustomize 5 for the tests.

Signed-off-by: Kimonas Sotirchos <kimwnasptd@arrikto.com>

* gh-action: Use the kustomize script in gh action

Update the GH Action that runs the unit test to use the same script we
use in the other GH Actions for fetching kustomize.

Signed-off-by: Kimonas Sotirchos <kimwnasptd@arrikto.com>

* Add FAQ for earlier versions of Kustomize

Signed-off-by: Kimonas Sotirchos <kimwnasptd@arrikto.com>

---------

Signed-off-by: Kimonas Sotirchos <kimwnasptd@arrikto.com>
google-oss-prow bot pushed a commit that referenced this pull request Mar 13, 2023
* example: Update for kustomize 5

Add the sortOptions in the kustomization.yaml to ensure we are creating
MutationWebhooks before Deployments and Pods. This way we'll ensure that
Pods will get Istio sidecars in a race-free way.



* readme: Update instructions to use Kustomize 5

Update instructions to inform users to use Kustomize 5. We'll also need
to do an awk command, to remove a line that ends up in the final output.
That is a regression on Kustomize's side. Once a newer version of
Kustomize is available with the fix we'll then remove this awk command.



* gh-action: Update to Kustomize 5

Update the helper script to use Kustomize 5 for the tests.



* gh-action: Use the kustomize script in gh action

Update the GH Action that runs the unit test to use the same script we
use in the other GH Actions for fetching kustomize.



* Add FAQ for earlier versions of Kustomize



---------

Signed-off-by: Kimonas Sotirchos <kimwnasptd@arrikto.com>
Co-authored-by: Kimonas Sotirchos <kimwnasptd@arrikto.com>
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.

3 participants