Skip to content

build: bump OCP_VERSION to 4.12#3458

Merged
jlebon merged 1 commit intocoreos:mainfrom
dustymabe:dusty-ocp-version
May 9, 2023
Merged

build: bump OCP_VERSION to 4.12#3458
jlebon merged 1 commit intocoreos:mainfrom
dustymabe:dusty-ocp-version

Conversation

@dustymabe
Copy link
Copy Markdown
Member

4.10 is a bit old at this point.

@dustymabe
Copy link
Copy Markdown
Member Author

should we add bumping this to an SOP?

@jlebon
Copy link
Copy Markdown
Member

jlebon commented May 8, 2023

I'd vote for just stopping pinning and always pulling the latest stable version? https://mirror.openshift.com/pub/openshift-v4/clients/ocp/latest/openshift-client-linux.tar.gz is an unversioned link.

Also relatedly, I just noticed that we're doing this on all arches, but we're always downloading the x86_64 one. Not all arches are available on the mirrors so we should probably gate the fetching logic to only x86_64.

@dustymabe
Copy link
Copy Markdown
Member Author

dustymabe commented May 8, 2023

I'd vote for just stopping pinning and always pulling the latest stable version? https://mirror.openshift.com/pub/openshift-v4/clients/ocp/latest/openshift-client-linux.tar.gz is an unversioned link.

I think that makes sense especially for latest COSA/FCOS/RHCOS devel, but when we branch COSA maybe it makes sense to freeze on one? Do we know what this is really used for in the pipeline(s) today?

Also relatedly, I just noticed that we're doing this on all arches, but we're always downloading the x86_64 one. Not all arches are available on the mirrors so we should probably gate the fetching logic to only x86_64.

+1

@jlebon
Copy link
Copy Markdown
Member

jlebon commented May 8, 2023

I'd vote for just stopping pinning and always pulling the latest stable version? mirror.openshift.com/pub/openshift-v4/clients/ocp/latest/openshift-client-linux.tar.gz is an unversioned link.

I think that makes sense especially for latest COSA/FCOS/RHCOS devel, but when we branch COSA maybe it makes sense to freeze on one? Do we know what this is really used for in the pipeline(s) today?

It was initially added for testing the layering stuff in Prow (#2777), but I'm not sure if that test still exists today (the bit it linked to is no longer there). This script uses it, but I don't see anything on the openshift/release side calling that script. On the pipeline side, we use it for a single oc get. The bigger value honestly is probably that it's already in the container image for devs to use. Cool with pinning on branching, but not sure it's worth it given that we don't really use advanced features from it that would be more likely to regress.

@dustymabe
Copy link
Copy Markdown
Member Author

Also relatedly, I just noticed that we're doing this on all arches, but we're always downloading the x86_64 one. Not all arches are available on the mirrors so we should probably gate the fetching logic to only x86_64.

I took a new look at this. I think we are downloading it for the arches correctly given we are calling "$(arch)" in there:

curl -L https://mirror.openshift.com/pub/openshift-v4/"$(arch)"/clients/ocp/latest-$OCP_VERSION/openshift-client-linux.tar.gz | tar zxf - oc \

@jlebon
Copy link
Copy Markdown
Member

jlebon commented May 8, 2023

Also relatedly, I just noticed that we're doing this on all arches, but we're always downloading the x86_64 one. Not all arches are available on the mirrors so we should probably gate the fetching logic to only x86_64.

I took a new look at this. I think we are downloading it for the arches correctly given we are calling "$(arch)" in there:

curl -L https://mirror.openshift.com/pub/openshift-v4/"$(arch)"/clients/ocp/latest-$OCP_VERSION/openshift-client-linux.tar.gz | tar zxf - oc \

Ahh nice, missed that. I was confused by the fact that the final directory containing the artifacts (e.g. this one) nonetheless appears to have e.g. both x86_64 and aarch64 versions, but not the others. Likely for historical reasons?

@dustymabe
Copy link
Copy Markdown
Member Author

Likely for historical reasons?

yeah not sure.

jlebon
jlebon previously approved these changes May 9, 2023
Copy link
Copy Markdown
Member

@jlebon jlebon left a comment

Choose a reason for hiding this comment

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

Good with this too if we want to defer the unpinning to a follow-up.

cgwalters
cgwalters previously approved these changes May 9, 2023
@dustymabe
Copy link
Copy Markdown
Member Author

Good with this too if we want to defer the unpinning to a follow-up.

i'll unpin here will get back to it soon.

@dustymabe dustymabe dismissed stale reviews from cgwalters and jlebon via ac02846 May 9, 2023 19:01
@dustymabe dustymabe force-pushed the dusty-ocp-version branch from ad44d7e to ac02846 Compare May 9, 2023 19:01
@dustymabe
Copy link
Copy Markdown
Member Author

updated

If we need to pin in the future we can just set the $OCP_VERSION
variable again.
@dustymabe dustymabe force-pushed the dusty-ocp-version branch from ac02846 to f2d54af Compare May 9, 2023 19:29
@jlebon jlebon enabled auto-merge (rebase) May 9, 2023 20:02
@jlebon jlebon merged commit f78ee0e into coreos:main May 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants