Skip to content

CORS-3724: bump go v1.23 and k8s dependencies#9396

Merged
patrickdillon merged 8 commits intoopenshift:mainfrom
tthvo:CORS-3724
Jan 28, 2025
Merged

CORS-3724: bump go v1.23 and k8s dependencies#9396
patrickdillon merged 8 commits intoopenshift:mainfrom
tthvo:CORS-3724

Conversation

@tthvo
Copy link
Copy Markdown
Member

@tthvo tthvo commented Jan 23, 2025

Release checklist: bump go v1.23 and update k8s deps (i.e. v0.32.1)
Note: k8s.io/api@v0.32.1 requires bumps go version to 1.23.
Commands:

go mod edit --require=k8s.io/api@v0.32.1
go mod edit --require=k8s.io/apimachinery@v0.32.1
go mod edit --require=k8s.io/apiextensions-apiserver@v0.32.1
go mod edit --require=k8s.io/cloud-provider-vsphere@v0.32.1
go mod tidy
go mod vendor

@openshift-ci-robot
Copy link
Copy Markdown
Contributor

openshift-ci-robot commented Jan 23, 2025

@tthvo: This pull request references CORS-3724 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 sub-task to target the "4.19.0" version, but no target version was set.

Details

In response to this:

Release checklist: bump go v1.23 and update k8s deps (i.e. v0.32.1)
Note: k8s.io/api@v0.32.1 requires bumps go version to 1.23.
Commands:

go mod edit --require=k8s.io/api@v0.32.1
go mod edit --require=k8s.io/apimachinery@v0.32.1
go mod edit --require=k8s.io/apiextensions-apiserver@v0.32.1
go mod edit --require=k8s.io/cloud-provider-vsphere@v0.32.1
go mod tidy
go mod vendor

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-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Jan 23, 2025
@openshift-ci openshift-ci Bot requested review from barbacbd and sadasu January 23, 2025 19:27
@tthvo
Copy link
Copy Markdown
Member Author

tthvo commented Jan 23, 2025

/cc @patrickdillon

@openshift-ci openshift-ci Bot requested a review from patrickdillon January 23, 2025 19:30
@tthvo
Copy link
Copy Markdown
Member Author

tthvo commented Jan 23, 2025

/cc @2uasimojo

@openshift-ci openshift-ci Bot requested a review from 2uasimojo January 23, 2025 19:38
@patrickdillon
Copy link
Copy Markdown
Contributor

/cc @2uasimojo

@2uasimojo this is just a heads up, we have a note to let hive know whenver we bump k8s dependencies

@patrickdillon
Copy link
Copy Markdown
Contributor

@tthvo we can see in the test failures that some of the images are failing to build because some of the images are still on 1.22:

FROM registry.ci.openshift.org/ocp/builder:rhel-9-golang-1.22-openshift-4.17 AS builder

FROM registry.ci.openshift.org/ocp/builder:rhel-9-golang-1.22-openshift-4.17 AS builder

You should be able to bump those in this PR

I just merged #9275 which would update the altinfra images, so don't bother with those

@tthvo
Copy link
Copy Markdown
Member Author

tthvo commented Jan 23, 2025

@tthvo we can see in the test failures that some of the images are failing to build because some of the images are still on 1.22

ack. Thanks! Bumped those images and rebased.

@tthvo
Copy link
Copy Markdown
Member Author

tthvo commented Jan 23, 2025

I think we might also need to bump golint in CI to use version that support gov1.23 (that should be >= v1.60.1)? See: https://golangci-lint.run/product/changelog/#v1601

@tthvo
Copy link
Copy Markdown
Member Author

tthvo commented Jan 24, 2025

/retest

@patrickdillon
Copy link
Copy Markdown
Contributor

I think we might also need to bump golint in CI to use version that support gov1.23 (that should be >= v1.60.1)? See: https://golangci-lint.run/product/changelog/#v1601

openshift/release#60961

@tthvo
Copy link
Copy Markdown
Member Author

tthvo commented Jan 24, 2025

openshift/release#60961

It seems later version of golint detects additional lint errors (See error from rehearsal. Would you like me to include the changes in this PR too since we are bumping go version here?

@patrickdillon
Copy link
Copy Markdown
Contributor

Would you like me to include the changes in this PR too since we are bumping go version here?

Yeah if you could take a stab at fixing those lint errors and include those in this PR that would be great.

@barbacbd
Copy link
Copy Markdown
Contributor

@tthvo Lets run a fixup on your last couple commits. Looks like you added a comment but we need to edit the commit for fixup too.

tthvo added 6 commits January 24, 2025 13:31
The file .golangci.yaml contains configurations for golangci-lint.
As golangci-lint version is bumped, some configurations are deprecated
and moved/removed. With this patch, the file is now compatible.

For gosec, we can safely exclude G115 (integer overflow converting between int types).
For errcheck, we skip checking type assertions.
@tthvo
Copy link
Copy Markdown
Member Author

tthvo commented Jan 24, 2025

Yeah if you could take a stab at fixing those lint errors and include those in this PR that would be great.

@patrickdillon The fixes are included in the recent commits :D I do have to adjust the .golangci.yaml to be compatible with later version of the lint tool.

Lets run a fixup on your last couple commits. Looks like you added a comment but we need to edit the commit for fixup too.

@barbacbd Thanks! I editted the commit for fixup now :D

@patrickdillon
Copy link
Copy Markdown
Contributor

/approve

This LGTM. I think golint is failing because openshift/release#60961 hasn't merged yet. I just put the labels on it, so it should merge quickly. We can then rerun golint.

This LGTM pending tests.

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Jan 24, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: patrickdillon

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:

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

@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 24, 2025
@tthvo
Copy link
Copy Markdown
Member Author

tthvo commented Jan 25, 2025

/retest

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Jan 25, 2025
@patrickdillon
Copy link
Copy Markdown
Contributor

golint is failing again as CI mirrored latest golint image is now pointing to v1.64.3 (i.e. it was v1.62.2 earlier - I used this version) . I included additional lint fixes in the latest commit. That should fix it :D

Oh, good catch. In that case, I think we should pin the version in CI. I thought using latest would be less hassle for us, but it seems backwards compatibility is not a given with golint, and we don't want a version bump popping up and breaking our golint tests unexpectedly while we're working on other things.

@tthvo will you fix what I did in openshift/release#60961 and pin to v1.64.3 (or another version of your choice)?

@patrickdillon
Copy link
Copy Markdown
Contributor

hm edge zones test is failing due to failure to remove bootstrap ssh rules, not an issue with this pr, but this is a bug we thought we fixed: https://prow.ci.openshift.org/view/gs/test-platform-results/pr-logs/pull/openshift_installer/9396/pull-ci-openshift-installer-main-e2e-aws-ovn-edge-zones/1883011929534894080

I will investigate and open a new bug

@patrickdillon
Copy link
Copy Markdown
Contributor

/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 Jan 25, 2025
@openshift-ci-robot
Copy link
Copy Markdown
Contributor

/retest-required

Remaining retests: 0 against base HEAD d224d2c and 2 for PR HEAD e5738d5 in total

2 similar comments
@openshift-ci-robot
Copy link
Copy Markdown
Contributor

/retest-required

Remaining retests: 0 against base HEAD d224d2c and 2 for PR HEAD e5738d5 in total

@openshift-ci-robot
Copy link
Copy Markdown
Contributor

/retest-required

Remaining retests: 0 against base HEAD d224d2c and 2 for PR HEAD e5738d5 in total

@tthvo
Copy link
Copy Markdown
Member Author

tthvo commented Jan 27, 2025

@tthvo will you fix what I did in openshift/release#60961 and pin to v1.64.3 (or another version of your choice)?

Right, I opened the PR here: openshift/release#61015. I will push another commit in this PR to pin our local version in hack scripts to 1.63.4 too.

hm edge zones test is failing due to failure to remove bootstrap ssh rules, not an issue with this pr, but this is a bug we thought we fixed: https://prow.ci.openshift.org/view/gs/test-platform-results/pr-logs/pull/openshift_installer/9396/pull-ci-openshift-installer-main-e2e-aws-ovn-edge-zones/1883011929534894080

I was wondering if it was related. Thanks for the explanation!

@openshift-ci openshift-ci Bot removed the lgtm Indicates that a PR is ready to be merged. label Jan 27, 2025
@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Jan 27, 2025

@tthvo: 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-nutanix-ovn 822f828 link false /test e2e-nutanix-ovn
ci/prow/e2e-azure-ovn-shared-vpc 822f828 link false /test e2e-azure-ovn-shared-vpc
ci/prow/e2e-agent-compact-ipv4-appliance-diskimage 822f828 link false /test e2e-agent-compact-ipv4-appliance-diskimage
ci/prow/azure-ovn-marketplace-images 822f828 link false /test azure-ovn-marketplace-images
ci/prow/e2e-vsphere-ovn-upi-zones 822f828 link false /test e2e-vsphere-ovn-upi-zones
ci/prow/e2e-vsphere-host-groups-ovn-custom-no-upgrade 822f828 link false /test e2e-vsphere-host-groups-ovn-custom-no-upgrade
ci/prow/e2e-aws-ovn-shared-vpc-edge-zones 822f828 link false /test e2e-aws-ovn-shared-vpc-edge-zones
ci/prow/okd-scos-e2e-aws-ovn 822f828 link false /test okd-scos-e2e-aws-ovn
ci/prow/e2e-ibmcloud-ovn 822f828 link false /test e2e-ibmcloud-ovn
ci/prow/e2e-openstack-dualstack-upi 822f828 link false /test e2e-openstack-dualstack-upi
ci/prow/e2e-azure-ovn-resourcegroup 822f828 link false /test e2e-azure-ovn-resourcegroup
ci/prow/e2e-openstack-nfv-intel 822f828 link false /test e2e-openstack-nfv-intel
ci/prow/e2e-azurestack 822f828 link false /test e2e-azurestack
ci/prow/e2e-metal-single-node-live-iso 822f828 link false /test e2e-metal-single-node-live-iso
ci/prow/e2e-openstack-proxy 822f828 link false /test e2e-openstack-proxy
ci/prow/e2e-metal-assisted 822f828 link false /test e2e-metal-assisted

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-sigs/prow repository. I understand the commands that are listed here.

@patrickdillon
Copy link
Copy Markdown
Contributor

/lgtm

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Jan 28, 2025
@patrickdillon
Copy link
Copy Markdown
Contributor

/skip

@openshift-ci-robot
Copy link
Copy Markdown
Contributor

/retest-required

Remaining retests: 0 against base HEAD 77a5cc0 and 2 for PR HEAD 822f828 in total

@patrickdillon
Copy link
Copy Markdown
Contributor

/override ci/prow/e2e-azure-ovn-upi

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Jan 28, 2025

@patrickdillon: Overrode contexts on behalf of patrickdillon: ci/prow/e2e-azure-ovn-upi

Details

In response to this:

/override ci/prow/e2e-azure-ovn-upi

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-sigs/prow repository.

@patrickdillon
Copy link
Copy Markdown
Contributor

If this gets stuck I'm inclined to green button merge it. It has enough tests passing that I'm not concerned--although I will wait to see images and integration-tests pass

@patrickdillon
Copy link
Copy Markdown
Contributor

/skip

@patrickdillon
Copy link
Copy Markdown
Contributor

/override ci/prow/e2e-azure-ovn-upi

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Jan 28, 2025

@patrickdillon: Overrode contexts on behalf of patrickdillon: ci/prow/e2e-azure-ovn-upi

Details

In response to this:

/override ci/prow/e2e-azure-ovn-upi

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-sigs/prow repository.

@patrickdillon patrickdillon merged commit fb88402 into openshift:main Jan 28, 2025
@tthvo tthvo deleted the CORS-3724 branch January 28, 2025 22:15
@openshift-bot
Copy link
Copy Markdown
Contributor

[ART PR BUILD NOTIFIER]

Distgit: ose-installer-altinfra
This PR has been included in build ose-installer-altinfra-container-v4.19.0-202501282309.p0.gfb88402.assembly.stream.el9.
All builds following this will include this PR.

@openshift-bot
Copy link
Copy Markdown
Contributor

[ART PR BUILD NOTIFIER]

Distgit: ose-baremetal-installer
This PR has been included in build ose-baremetal-installer-container-v4.19.0-202501282309.p0.gfb88402.assembly.stream.el9.
All builds following this will include this PR.

@openshift-bot
Copy link
Copy Markdown
Contributor

[ART PR BUILD NOTIFIER]

Distgit: ose-installer-terraform-providers
This PR has been included in build ose-installer-terraform-providers-container-v4.19.0-202501282309.p0.gfb88402.assembly.stream.el9.
All builds following this will include this PR.

@openshift-bot
Copy link
Copy Markdown
Contributor

[ART PR BUILD NOTIFIER]

Distgit: ose-installer-artifacts
This PR has been included in build ose-installer-artifacts-container-v4.19.0-202501282309.p0.gfb88402.assembly.stream.el9.
All builds following this will include this PR.

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.

5 participants