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

fix: Tolerate kustomize version 3.2 #233

Merged
merged 2 commits into from
Sep 9, 2022

Conversation

ckadner
Copy link
Member

@ckadner ckadner commented Sep 8, 2022

Motivation

Developers who work on both Kubeflow and ModelMesh must use different versions of the kustomize CLI as Kubeflow installation requires version no greater than 3.2.0 yet ModelMesh installation requires a version greater than 4.0.0.

There have been previous changes to the install scripts attempting to tolerate older kustomize versions, yet for 3.2 it is still causing errors like these:

...
Installing ModelMesh Serving built-in runtimes
Error: unknown flag: --load-restrictor
error: no objects passed to apply

This is because various versions of kustomize produce differently formatted output and the ModelMesh install scripts do not capture the intended semantic version identifier, i.e.

$ kustomize version  # 3.2

Version: {KustomizeVersion:3.2.0 GitCommit:a3103f1e62ddb5b696daa3fd359bb6f2e8333b49 BuildDate:2019-09-18T16:26:36Z GoOs:darwin GoArch:amd64}

$ kustomize version | awk -F'kustomize/' '{print $2}'


# notice nothing here
$ kustomize version  # 3.4

{Version:kustomize/v3.4.0 GitCommit:2c9635967a2b1469d605a91a1d040bd27c73ca7d BuildDate:2019-11-12T05:00:57Z GoOs:darwin GoArch:amd64}

$ kustomize version | awk -F'kustomize/' '{print $2}'

v3.4.0 GitCommit:2c9635967a2b1469d605a91a1d040bd27c73ca7d BuildDate:2019-11-12T05:00:57Z GoOs:darwin GoArch:amd64}

Modifications

Change the install (and delete) scripts to grep only the semantic version and use that to choose the correct install command syntax.

Result

Installation (and de-installation) works for kustomize versions 3.2.0 up to latest (currently 4.5.7)

Related Issues:

Fixes #66

Fixes kserve#66

Signed-off-by: Christian Kadner <[email protected]>
@ckadner ckadner requested a review from njhill September 8, 2022 21:36
@ckadner ckadner added the bug Something isn't working label Sep 8, 2022
@ckadner
Copy link
Member Author

ckadner commented Sep 8, 2022

/restest

@ckadner
Copy link
Member Author

ckadner commented Sep 8, 2022

@njhill -- I seem to not have much luck with FVT tests :-)

It does not seem to be related to my script changes.

Copy link
Member

@njhill njhill left a comment

Choose a reason for hiding this comment

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

Thanks @ckadner. The tests passed after being re-run. I have a feeling the failures were related to the tests being run from multiple PRs concurrently. I don't remember if we have any mutual exclusion for this.

@kserve-oss-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ckadner, njhill

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

@njhill
Copy link
Member

njhill commented Sep 9, 2022

/lgtm

@kserve-oss-bot kserve-oss-bot merged commit 5bf7092 into kserve:main Sep 9, 2022
@ckadner
Copy link
Member Author

ckadner commented Sep 9, 2022

Thanks @njhill

ckadner added a commit to ckadner/modelmesh-serving that referenced this pull request Sep 15, 2022
Until ModelMesh release 1.0 the Quickstart guide should
require kustomize version 4.0.0 since the ModelMesh 0.9
release does not yet have the install script fixes (kserve#233)
to tolerate older versions of the kustomize CLI.

After ModelMesh release 1.0 the Quickstart should be changed
to allow kustomize version 3.2.0 since that next release
will include the respective install script fixes to tolerate
older versions of kustomize.

Fixes kserve#243

Signed-off-by: Christian Kadner <[email protected]>
kserve-oss-bot pushed a commit that referenced this pull request Sep 15, 2022
#### Motivation

Until ModelMesh release 1.0 the [Quickstart guide](https://github.com/kserve/modelmesh-serving/blob/main/docs/quickstart.md) should require `kustomize` version `4.0.0`+ since the ModelMesh 0.9 release does not yet have the [install script fixes](#233) to tolerate older versions of the `kustomize` CLI.

After the ModelMesh 1.0 release, the Quickstart doc should be changed to allow `kustomize` version `3.2.0` since that next release will include the respective install script fixes (#233) to tolerate older versions of `kustomize`.

Fixes #243

#### Modifications

Reinstate the `kustomize` v`4.0.0`+ prereqs in the Quickstart guide.

#### Result

The required version of `kustomize` in the Quickstart guide matches what the install and delete scripts support.

#### TODOs

Create a new issue to change the Quickstart guide to allow `kustomize` version `3.2.0` after ModelMesh release `1.0` since that next release will include the respective install script fixes (#233) to tolerate older versions of `kustomize`.

/cc @njhill @rafvasq

Signed-off-by: Christian Kadner <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved bug Something isn't working lgtm
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Installer from quick start guide yields an error
3 participants