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

MCO-909: Openshift/Kubernetes 1.29 Rebase Updates #4256

Merged
merged 1 commit into from Mar 14, 2024

Conversation

dkhater-redhat
Copy link
Contributor

- What I did
Update the go.mod, go.sum and vendor dependencies pointing to the kube1.29 libraries. This includes all direct kubernetes related libraries as well as openshift/api , openshift/client-go, openshift/library-go and openshift/runtime-utils.
- How to verify it

- Description for the changelog

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Mar 12, 2024
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Mar 12, 2024

@dkhater-redhat: This pull request references MCO-909 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.16.0" version, but no target version was set.

In response to this:

- What I did
Update the go.mod, go.sum and vendor dependencies pointing to the kube1.29 libraries. This includes all direct kubernetes related libraries as well as openshift/api , openshift/client-go, openshift/library-go and openshift/runtime-utils.
- How to verify it

- Description for the changelog

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 openshift-eng/jira-lifecycle-plugin repository.

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 12, 2024
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.

This looks great.
As part of story, we should also update openshift/library-go, openshift/api, openshift/runtime-utils and openshift/client-go so that we have openshift specific libs updated as well

@dkhater-redhat
Copy link
Contributor Author

/retest

Copy link
Contributor

openshift-ci bot commented Mar 14, 2024

@dkhater-redhat: 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/okd-scos-e2e-aws-ovn 68368e0 link false /test okd-scos-e2e-aws-ovn

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.

@sinnykumari
Copy link
Contributor

Thanks for bumping OpenShift specific deps as well. This looks great. ci tests are passing, shouldn't need additional QE testing.
/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Mar 14, 2024
Copy link
Contributor

openshift-ci bot commented Mar 14, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dkhater-redhat, sinnykumari

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 [dkhater-redhat,sinnykumari]

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

@djoshy
Copy link
Contributor

djoshy commented Mar 14, 2024

Just a heads-up that that the api has had few commits over the past night: https://github.com/openshift/api/commits/master/
The latest bump breaks a feature gate call within kubelet=config controller, I ran into this while testing some API changes.
So if we are holding off on merging this, it might be best to bump it again(and fix the calls) while we wait for the label reqt to get removed

@sinnykumari
Copy link
Contributor

/hold
Thanks David for the bringing that up.
Is the breakage known and are getting fixed? Also, do you think this breakage will be caught-up by running any specific payload test on this PR?

@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 Mar 14, 2024
@djoshy
Copy link
Contributor

djoshy commented Mar 14, 2024

I couldn't built an MCO image with the api build - a function call arg/par list was changed, so we shouldn't need a payload test. Just trying to make the image locally(with bumping the api to github.com/openshift/api v0.0.0-20240314024039-4caef7fe3d0f should trigger the build failure

@sinnykumari
Copy link
Contributor

Umm, image ci test and other tests are green on this PR. Am I missing something?

@djoshy
Copy link
Contributor

djoshy commented Mar 14, 2024

This PR does not have the latest openshift api bump, the change I'm talking about happened somewhere in the last 3 commits(https://github.com/openshift/api/commits/master/)

@jkyros
Copy link
Contributor

jkyros commented Mar 14, 2024

This PR does not have the latest openshift api bump, the change I'm talking about happened somewhere in the last 3 commits(https://github.com/openshift/api/commits/master/)

Yeah, Dalia and I were horsing with it last night trying to accommodate Eads' new ClusterProfile-centric feature gate stuff in openshift/api#1788, but I don't know that the MCO has the plumbing right now to figure out the cluster profile without doing something like Eads did here: https://github.com/openshift/cluster-config-operator/pull/411/files#r1520245798 ?

(Dalia is on it, and is following up, I'm just sharing it here to avoid duplicate investigations)

The tests are green here because I believe this moves the API version back to github.com/openshift/api v0.0.0-20240311132450-dcd6ab38a0f7 for now to avoid the feature gate commits from the last two days.

@djoshy
Copy link
Contributor

djoshy commented Mar 14, 2024

Ah, thanks for the addn context John, I was trying to get some other API stuff working when I ran into it. I'm okay with merging as is if we want to override w the additional tag, but if we are waiting might as well try to patch it up, to help with other API efforts

@sinnykumari
Copy link
Contributor

Ah, thanks John for the additional context. So, I think this should be good to go? We don't need latest changes for this bump. This is more of an exercise on what all changes we will need whenever a major version kube bump occurs in OCP.

@dkhater-redhat
Copy link
Contributor Author

Yes exactly what John said 😄

The latest work for Openshift/api adds functionality that breaks our helper and bootstrap files by looking for a current 'ClusterProfile'. MCO doesn't have the functionality yet to capture this information, and I am going to follow up with Joel or David Eads on this matter today. However for this PR, I thought that the latest api changes over the past few days were not needed at this time.

if everything is good, does the PR look good for merge?

@sinnykumari
Copy link
Contributor

sinnykumari commented Mar 14, 2024

Based on the conversation, this should be safe to get merged.
/hold cancel
This is not urgent, but don't see risk in getting it merged. @djoshy thoughts on adding acknowledge-critical-fixes-only ?

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 14, 2024
@djoshy
Copy link
Contributor

djoshy commented Mar 14, 2024

I think it's safe to merge (:
/label acknowledge-critical-fixes-only

@openshift-ci openshift-ci bot added the acknowledge-critical-fixes-only Indicates if the issuer of the label is OK with the policy. label Mar 14, 2024
@openshift-merge-bot openshift-merge-bot bot merged commit 4a4196f into openshift:master Mar 14, 2024
17 of 18 checks passed
@dkhater-redhat dkhater-redhat deleted the mco-909 branch March 14, 2024 20:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
acknowledge-critical-fixes-only Indicates if the issuer of the label is OK with the policy. approved Indicates a PR has been approved by an approver from all required OWNERS files. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. 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

5 participants