Skip to content

Make OKD/SCOS Dockerfile regexes match again after rhel-coreos image name change #3597

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.

Sign up for GitHub

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

Jump to bottom
Merged
openshift-merge-robot merged 1 commit into from
Mar 9, 2023
Merged

Make OKD/SCOS Dockerfile regexes match again after rhel-coreos image name change #3597

openshift-merge-robot merged 1 commit into from
Mar 9, 2023

Conversation

@jkyros
Copy link
Member

@jkyros jkyros commented Mar 9, 2023

We broke OKD, SCOS and the required okd-scos-images test when #3596 merged because our regexes no longer matched without the number.

This can go in with something else, we just need to fix it 😄

@jkyros
Dockerfile: Fix removing extensions for fcos/scos
cb2958d 
Last minute change in openshift#3496 resulted in the number being removed from
the end of the `rhel-coros-8/9` image, it is now just simply in there as
`rhel-coreos`, and as a result the regex that was scraping out the
extensions images (because fcos/scos dont' ship them) no longer works.

This adjusts the sed command in the Dockerfile so it matches again now
that the number is missing, and the extensions are properly removed.
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 9, 2023
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Mar 9, 2023

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 9, 2023
@jkyros
Copy link
Member Author

jkyros commented Mar 9, 2023

/test okd-scos-images

@jkyros
Copy link
Member Author

jkyros commented Mar 9, 2023

Yep, looks like that fixed it.

yuqi-zhang
yuqi-zhang approved these changes Mar 9, 2023
View reviewed changes
Copy link
Contributor

@yuqi-zhang yuqi-zhang left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Mar 9, 2023
@jkyros jkyros marked this pull request as ready for review March 9, 2023 02:14
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 9, 2023
@openshift-ci openshift-ci bot requested review from sinnykumari and yuqi-zhang March 9, 2023 02:15
@yuqi-zhang
Copy link
Contributor

yuqi-zhang commented Mar 9, 2023 •
edited
Loading

Shouldn't block this, but the OKD-SCOS upgrade job sees it too: https://prow.ci.openshift.org/view/gs/origin-ci-test/pr-logs/pull/openshift_machine-config-operator/3597/pull-ci-openshift-machine-config-operator-master-okd-scos-e2e-gcp-ovn-upgrade/1633652495685259264

Is that a different manifestation? Or is it because the previous one doesn't have it when it tries to install and old one for upgrade?

@jkyros
Copy link
Member Author

jkyros commented Mar 9, 2023 •
edited
Loading

Or is it because the previous one doesn't have it when it tries to install and old one for upgrade?

Yep, I think that's it. Looking at what it was doing, the MCO it's using from the release-initial image stream that's failing is:

registry.ci.openshift.org/origin/scos-4.14@sha256:4824d6956b6e55b98e20c3e11189cd3969816519b45c19c88c7a9004ccab2d59

And that image does still contain the extensions references:

- name: centos-stream-coreos-9
    from:
      kind: DockerImage
      name: placeholder.url.oc.will.replace.this.org/placeholdernamespace:centos-stream-coreos-9
  - name: centos-stream-coreos-9-extensions
    from:
      kind: DockerImage
      name: placeholder.url.oc.will.replace.this.org/placeholdernamespace:centos-stream-coreos-9-extensions

But the one it builds from this PR (that it's going to try to upgrade to) is successful.

I mean that's maybe a fair assumption that the current image available is sane (as long as....er...nobody bypassed CI to get it in there 😄 )

@sdodson
Copy link
Member

sdodson commented Mar 9, 2023 •
edited
Loading

Thanks for so quickly fixing this up! Should this also be included in the PR in which we make the same changes to the release-4.13 branch?
CC: @cgwalters

@jkyros
Copy link
Member Author

jkyros commented Mar 9, 2023 •
edited
Loading

Should this also be included in the PR in which we make the same changes to the release-4.13 branch?

Yes, it should. If we don't, we'll have the same issues there.

@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Mar 9, 2023

/retest-required

Remaining retests: 0 against base HEAD 832de9e and 2 for PR HEAD cb2958d in total

@jkyros
Copy link
Member Author

jkyros commented Mar 9, 2023

well this definitely didn't break that test
/test e2e-aws-ovn-upgrade

@sinnykumari
Copy link
Contributor

sinnykumari commented Mar 9, 2023

/test e2e-aws-ovn-upgrade

@cgwalters
Copy link
Member

cgwalters commented Mar 9, 2023

/test e2e-aws-ovn-upgrade
Should be fixed now

@cgwalters
Copy link
Member

cgwalters commented Mar 9, 2023

/lgtm

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Mar 9, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cgwalters, jkyros, yuqi-zhang

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:
  • OWNERS [cgwalters,jkyros,yuqi-zhang]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@cgwalters
Copy link
Member

cgwalters commented Mar 9, 2023

The e2e-aws-ovn-upgrade job failed on apiserver disruption. But a release image got built, so I'm inclined to add an /override for this - any objections?

@jkyros
Copy link
Member Author

jkyros commented Mar 9, 2023

Agree, this isn't affecting the disruption. No objection.

@cgwalters
Copy link
Member

cgwalters commented Mar 9, 2023

/override ci/prow/e2e-aws-ovn-upgrade

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Mar 9, 2023

@cgwalters: Overrode contexts on behalf of cgwalters: ci/prow/e2e-aws-ovn-upgrade

Details

In response to this:

/override ci/prow/e2e-aws-ovn-upgrade

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Mar 9, 2023

@jkyros: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-hypershift cb2958d link false /test e2e-hypershift
ci/prow/okd-scos-e2e-gcp-ovn-upgrade cb2958d link false /test okd-scos-e2e-gcp-ovn-upgrade
ci/prow/e2e-alibabacloud-ovn cb2958d link false /test e2e-alibabacloud-ovn

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@openshift-merge-robot openshift-merge-robot merged commit 7b3939c into openshift:master Mar 9, 2023
@cgwalters cgwalters mentioned this pull request Mar 10, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sinnykumari sinnykumari Awaiting requested review from sinnykumari

@yuqi-zhang yuqi-zhang Awaiting requested review from yuqi-zhang

Assignees

@cgwalters cgwalters

@yuqi-zhang yuqi-zhang

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@jkyros @yuqi-zhang @sdodson @openshift-ci-robot @sinnykumari @cgwalters @openshift-merge-robot