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

Bump openshift/api #3336

Merged
merged 4 commits into from Sep 20, 2022
Merged

Conversation

pierreprinetti
Copy link
Member

@pierreprinetti pierreprinetti commented Sep 14, 2022

This bump is instrumental to the development of OpenStack FailureDomains.

@yuqi-zhang
Copy link
Contributor

The current api bump is being performed via #3269

Let's rebase on that as necessary

/hold

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 14, 2022
@sinnykumari
Copy link
Contributor

openshift/api bump has been done with PR #3269 merged. If you still need further api bump, please rebase this PR on top of master to get required changes

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 16, 2022
@sinnykumari sinnykumari mentioned this pull request Sep 16, 2022
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 16, 2022
@pierreprinetti
Copy link
Member Author

@yuqi-zhang I am not sure what to do with that failing verify test. Would you know?

@pierreprinetti
Copy link
Member Author

@yuqi-zhang passed make verify by bumping golangci-lint. PTAL!

go.mod Outdated Show resolved Hide resolved
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.

Hey, sorry I didn't get to this earlier. It looks like the kubevirt platform type was updated, namely this:
https://github.com/openshift/machine-config-operator/blob/master/pkg/controller/template/render_test.go#L86

Should be external now, and it should pass

Otherwise, generally seems fine

Copy link
Contributor

@sinnykumari sinnykumari left a comment

Choose a reason for hiding this comment

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

LGTM, Thanks for the PR

@sinnykumari
Copy link
Contributor

Since #3269 has been merged, hold is not needed
/hold cancel

@openshift-ci openshift-ci bot added approved Indicates a PR has been approved by an approver from all required OWNERS files. and removed do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. labels Sep 20, 2022
@yuqi-zhang
Copy link
Contributor

hmm, the status is outdated, just to check

/test bootstrap-unit

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Sep 20, 2022

@pierreprinetti: The following test 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-openstack 4367a1c link false /test e2e-openstack

Full PR test history. Your PR dashboard.

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.

@yuqi-zhang
Copy link
Contributor

Ok, looks like unit tests are good now. Do you expect this to affect any specific platform? Should we run some additional tests (e.g. payload tests) to make sure?

And also to check, we expect e2e-openstack to fail right?

@pierreprinetti
Copy link
Member Author

we expect e2e-openstack to fail right?

All OpenStack CI 4.12 tests are failing at the moment unfortunately.

Do you expect this to affect any specific platform?

I am asking for this bump because I need to test an MCO patch against a PR of openshift/api that is based on a recent commit of openshift/api, and I am trying to avoid conflicts. We will need to bump again openshift/api in MCO when the work is ready.

Does that work for you?

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.

Thanks for the context. I think we have enough time to fix any issues. Since this is only a bump, we should be ok

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Sep 20, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Sep 20, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: pierreprinetti, sinnykumari, yuqi-zhang

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:
  • OWNERS [sinnykumari,yuqi-zhang]

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

@openshift-merge-robot openshift-merge-robot merged commit 26eaa2c into openshift:master Sep 20, 2022
@pierreprinetti pierreprinetti deleted the bump_api branch September 21, 2022 07:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
Development

Successfully merging this pull request may close these issues.

None yet

4 participants