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

chore(deps): bump k8s libs from 0.29.6 to 0.30.4 #19074

Merged
merged 12 commits into from
Aug 23, 2024

Conversation

crenshaw-dev
Copy link
Member

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
Copy link

bunnyshell bot commented Jul 15, 2024

❌ Preview Environment deleted from Bunnyshell

Available commands (reply to this comment):

  • 🚀 /bns:deploy to deploy the environment

Copy link

bunnyshell bot commented Jul 15, 2024

❌ Preview Environment deleted from Bunnyshell

Available commands (reply to this comment):

  • 🚀 /bns:deploy to deploy the environment

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
@sivchari
Copy link
Contributor

The option of go-to-protobuf may be changed, so this CI would fail. This problem occur in #18506 , too.

@crenshaw-dev
Copy link
Member Author

@sivchari ah gotcha. Want to just update that PR, and I'll close this one?

@sivchari
Copy link
Contributor

@crenshaw-dev
Sure. I want to progress the way to resolve the issue in #18506 with us. Perhaps, this problem is just resolved to solve the go-to-protobuf.

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
… 0.30.x

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
@crenshaw-dev
Copy link
Member Author

Codegen failure:

+ go-to-protobuf --go-header-file=/home/runner/go/src/github.com/argoproj/argo-cd/hack/custom-boilerplate.go.txt --packages=github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1 --apimachinery-packages=+k8s.io/apimachinery/pkg/util/intstr,+k8s.io/apimachinery/pkg/api/resource,+k8s.io/apimachinery/pkg/runtime/schema,+k8s.io/apimachinery/pkg/runtime,k8s.io/apimachinery/pkg/apis/meta/v1,k8s.io/api/core/v1,k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1 --proto-import=/home/runner/go/src/github.com/argoproj/argo-cd/vendor --proto-import=/home/runner/go/src/github.com/argoproj/argo-cd/dist/protoc-include --output-dir=/home/runner/go/src/
2024/07/19 14:28:27 protoc -I . -I /home/runner/go/src/ -I /home/runner/go/src/github.com/argoproj/argo-cd/vendor -I /home/runner/go/src/github.com/argoproj/argo-cd/dist/protoc-include --gogo_out=/home/runner/go/src/ /home/runner/go/src/k8s.io/api/core/v1/generated.proto
2024/07/19 14:28:27 Could not make proto path relative: /home/runner/go/src/k8s.io/api/core/v1/generated.proto: No such file or directory

Caused by this change in go-to-protobuf: kubernetes/code-generator@e52957c

Not sure yet how to fix it.

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
@crenshaw-dev
Copy link
Member Author

I just modified the script to copy dependencies from the vendor directory to the location go-to-protobuf expects them to be at.

@crenshaw-dev crenshaw-dev marked this pull request as ready for review August 19, 2024 22:02
@crenshaw-dev crenshaw-dev requested a review from a team as a code owner August 19, 2024 22:02
Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
Copy link

codecov bot commented Aug 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Please upload report for BASE (master@de53d8e). Learn more about missing BASE report.
Report is 31 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##             master   #19074   +/-   ##
=========================================
  Coverage          ?   55.86%           
=========================================
  Files             ?      316           
  Lines             ?    43784           
  Branches          ?        0           
=========================================
  Hits              ?    24458           
  Misses            ?    16776           
  Partials          ?     2550           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@sivchari
Copy link
Contributor

Sorry for not being able to respond. Brilliant, thanks.

@crenshaw-dev
Copy link
Member Author

You're good! No real rush, just wanna sort it in time for 2.13.

@crenshaw-dev
Copy link
Member Author

Questions I may or may not have time to answer before pushing to get this merged:

  1. do we need the toolchain line in go.mod?
  2. why does go-to-protobuf modify the stuff in vendors (specifically, why does it delete generated.pb.go files, breaking codegen) so that we have to re-run go mod vendor?
  3. why is the violations exception list file now empty?

A lot of this codegen stuff is deep lore that goes back before my time, so I'm not sure how safe it is to ignore these oddities.

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
@crenshaw-dev
Copy link
Member Author

Toolchain line wasn't necessary.

@crenshaw-dev crenshaw-dev changed the title chore(deps): bump k8s libs from 0.29.6 to 0.30.2 chore(deps): bump k8s libs from 0.29.6 to 0.30.4 Aug 20, 2024
Copy link
Member

@agaudreault agaudreault left a comment

Choose a reason for hiding this comment

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

LGTM. Tested locally (not checked out in gopath) and it works with updated tools. But i don't know what the violation list is for. Maybe @jannfis know?

go.mod Outdated Show resolved Hide resolved
Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
Copy link
Member

@agaudreault agaudreault left a comment

Choose a reason for hiding this comment

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

LGTM. Future improvement on codegen can be done in other PRs

@crenshaw-dev crenshaw-dev enabled auto-merge (squash) August 23, 2024 16:22
@crenshaw-dev crenshaw-dev merged commit ddd9d6a into argoproj:master Aug 23, 2024
27 of 28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants