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

Update dockerfiles to use go v1.22 #2931

Merged
merged 8 commits into from
Aug 28, 2024
Merged

Conversation

aggarwal0009
Copy link
Contributor

@aggarwal0009 aggarwal0009 commented Aug 19, 2024

Reason for Change:
Upgrading dockerfiles to go v1.22 to support device plugin which introduces kubelet dependencies requiring go 1.22

Reference to device plugin PR: #2886

Issue Fixed:

Requirements:

Notes:

@aggarwal0009 aggarwal0009 requested a review from matmerr August 19, 2024 22:03
@aggarwal0009 aggarwal0009 added the cns Related to CNS. label Aug 19, 2024
@matmerr
Copy link
Member

matmerr commented Aug 19, 2024

1.23 is out, any reason not going all the way up?

https://go.dev/blog/go1.23

@rbtr
Copy link
Contributor

rbtr commented Aug 19, 2024

1.23 is out, any reason not going all the way up?

https://go.dev/blog/go1.23

we should probably wait for at least 1.23.1 in case they snuck any bugs in the .0, but this is a good idea. some nice new features in 1.23.

@aggarwal0009 8 files does not seem like enough compared to when we moved to 1.21

@aggarwal0009 aggarwal0009 changed the title Update dockerfilesto use go v1.22 Update dockerfiles to use go v1.22 Aug 19, 2024
@aggarwal0009 aggarwal0009 requested review from ZetaoZhuang and a team as code owners August 20, 2024 00:23
@aggarwal0009
Copy link
Contributor Author

1.23 is out, any reason not going all the way up?
https://go.dev/blog/go1.23

we should probably wait for at least 1.23.1 in case they snuck any bugs in the .0, but this is a good idea. some nice new features in 1.23.

@aggarwal0009 8 files does not seem like enough compared to when we moved to 1.21

Updated

@aggarwal0009
Copy link
Contributor Author

/azp run Azure Container Networking PR

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

rbtr
rbtr previously approved these changes Aug 20, 2024
Copy link
Contributor

@rbtr rbtr left a comment

Choose a reason for hiding this comment

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

I'll update the PR check requirements once this tests clean so it can be queued to merge

@rbtr rbtr mentioned this pull request Aug 20, 2024
4 tasks
miguelgoms
miguelgoms previously approved these changes Aug 20, 2024
@aggarwal0009
Copy link
Contributor Author

/azp run Azure Container Networking PR

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@aggarwal0009 aggarwal0009 dismissed stale reviews from miguelgoms and rbtr via 146bdd8 August 20, 2024 17:08
@aggarwal0009
Copy link
Contributor Author

/azp run Azure Container Networking PR

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@timraymond
Copy link
Member

#2930 depends on this (adding it to link the PRs)

@Azure Azure deleted a comment from aggarwal0009 Aug 22, 2024
rbtr
rbtr previously approved these changes Aug 22, 2024
@rbtr rbtr enabled auto-merge August 23, 2024 15:14
nairashu
nairashu previously approved these changes Aug 23, 2024
@aggarwal0009 aggarwal0009 dismissed stale reviews from nairashu and rbtr via 5acef5d August 23, 2024 23:27
@aggarwal0009
Copy link
Contributor Author

/azp run Azure Container Networking PR

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Contributor

@huntergregory huntergregory left a comment

Choose a reason for hiding this comment

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

Since this is modifying NPM, please run both NPM Azure Pipelines tests and merge once green. They are not “required checks” because GitHub does not support requiring checks that trigger conditionally (i.e. only when npm/ directory is modified)

/azp run NPM Conformance Tests
/azp run NPM Scale Test

@huntergregory
Copy link
Contributor

/azp run NPM Conformance Tests

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@huntergregory
Copy link
Contributor

/azp run NPM Scale Test

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@rbtr rbtr added this pull request to the merge queue Aug 26, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 26, 2024
@paulyufan2 paulyufan2 added this pull request to the merge queue Aug 26, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 27, 2024
@aggarwal0009 aggarwal0009 added this pull request to the merge queue Aug 27, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 27, 2024
@paulyufan2 paulyufan2 added this pull request to the merge queue Aug 27, 2024
Merged via the queue into master with commit 8359dbb Aug 28, 2024
33 checks passed
@paulyufan2 paulyufan2 deleted the ankaggar/update_go_v1_22 branch August 28, 2024 00:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cns Related to CNS.
Projects
None yet
Development

Successfully merging this pull request may close these issues.