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 to k8s 29 and revendor installer #2211

Conversation

2uasimojo
Copy link
Member

@2uasimojo 2uasimojo commented Feb 14, 2024

Bump k8s dependencies to .29.1

Revendor installer to 20240202175915-f168b97656bd

HIVE-2351
HIVE-2418

@2uasimojo
Copy link
Member Author

Note to reviewers: I did try to do these two things separately, but alas...

@openshift-ci openshift-ci bot requested review from dlom and suhanime February 14, 2024 00:25
@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 14, 2024
@2uasimojo 2uasimojo mentioned this pull request Feb 14, 2024
Copy link

codecov bot commented Feb 14, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (18a6e55) 58.32% compared to head (457a8d3) 58.31%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2211      +/-   ##
==========================================
- Coverage   58.32%   58.31%   -0.01%     
==========================================
  Files         182      182              
  Lines       25675    25697      +22     
==========================================
+ Hits        14974    14986      +12     
- Misses       9445     9453       +8     
- Partials     1256     1258       +2     
Files Coverage Δ
pkg/controller/machinepool/awsactuator.go 78.55% <ø> (ø)
...g/controller/machinepool/machinepool_controller.go 53.82% <ø> (+0.36%) ⬆️
pkg/imageset/updateinstaller.go 45.73% <ø> (ø)
pkg/resource/apply.go 0.00% <ø> (ø)
pkg/resource/kubeconfig_factory.go 0.00% <ø> (ø)
pkg/resource/restconfig_factory.go 0.00% <ø> (ø)
pkg/validating-webhooks/hive/v1/feature_gates.go 100.00% <ø> (ø)

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 15, 2024
Minor changes to accommodate dependency bumps.

NOTE: This does not stand alone. It needs the subsequent installer bump
commits.

HIVE-2351
- Fake client stopped populating TypeMeta in certain cases, which broke
a clusterrelocate test. Switch assertions to ignore TypeMeta and only
compare the parts of the objects we care about.
- Installer changed the shape of the input to the MachineSets generator.
Accommodate.
- The cluster-control-plane-machine-set-operator methods we are using to
match failure domains now require a couple of additional arguments:
  - A logger. (But not the same one we generally use. Fortunately, this
  isn't the first time we've encountered this issue -- we have a handy
  converter.)
  - The remote cluster's `infrastructure` object. Currently this is only
  used by the VSphere generator; but for future-proofing, we're always
  grabbing it and passing it through.

HIVE-2418
@2uasimojo 2uasimojo force-pushed the HIVE-2351-2418/k8s-29-and-installer-revendor branch from bd3b64f to 457a8d3 Compare February 15, 2024 18:53
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 15, 2024
Copy link
Contributor

@suhanime suhanime left a comment

Choose a reason for hiding this comment

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

/lgtm


// `go mod tidy` pulls in 0.19.0 from somewhere, but apiextensions-apiserver and apiserver both want this version.
// The latter breaks in hiveadmission otherwise.
replace github.com/google/cel-go => github.com/google/cel-go v0.17.7
Copy link
Contributor

Choose a reason for hiding this comment

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

So hiveadmission no longer works with the latest cel-go?(v0.19.0 is the latest fwic). I think we might need a tech-debt somewhere to go have a look and fix. It is bad enough that we have all these requires and replaces for installer, we should at least try to clean up our own laundry.

Copy link
Member Author

Choose a reason for hiding this comment

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

Not exactly. If I upgrade cel-go to 19, hiveadmission breaks, but it's because hiveadmission => calls a thing in apiextensions-apiserver => calls a thing in cel-go that relies on cel-go being old. To upgrade cel-go, we would need a newer apiextensions-apiserver that has been updated to use the newer cel-go.

When I ran into the problem, I simply went and looked at what version of cel-go is required in apiextensions-apiserver's go.mod, and for 0.29.1 (and in fact all the way up to current master) -- it's this one.

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

openshift-ci bot commented Feb 15, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 2uasimojo, suhanime

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

Copy link
Contributor

openshift-ci bot commented Feb 15, 2024

@2uasimojo: all tests passed!

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.

@openshift-merge-bot openshift-merge-bot bot merged commit ac85e5f into openshift:master Feb 15, 2024
11 checks passed
@2uasimojo 2uasimojo deleted the HIVE-2351-2418/k8s-29-and-installer-revendor branch February 15, 2024 22:40
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

3 participants