CHORE: bump controller-gen/envtest and set --use-deprecated-gcs in Makefile recipe to false #281
Conversation
|
Requesting review: @IrvingMg @hasanawad94 |
IrvingMg
left a comment
There was a problem hiding this comment.
Thank you for sending the fix. However, I’m not sure I fully understood the issue. I see that you encountered an error while running make test locally as well, but was it the same issue that appeared in CI?
Also, what about the unauthorized error? Was it related to this issue in any way?
It would be great to better understand the root cause, in case there’s something we can do to prevent this from happening again in the future or in the other repositories.
|
Yep, @IrvingMg i saw the same unauthorized issue while i was running
which was solved by bumping the controller-gen version And the second one is this, In my understanding using I made a ad hoc fix by searching here and there. surprisingly it worked |
|
It’s interesting, I don’t see the following error in the CI logs: Specifically, I don’t see it in the CI run here: I assume there may be some environment differences. I’m trying to understand the difference so we can keep CI and local environments aligned. Do you have any idea what might be different? For example, could the CI images be outdated? Also, could you share the full logs from your local |
|
Thank you so much for keeping up and helping me with this issue @IrvingMg Here are the local |
|
Surprisingly after bumping the Now tests seem to work perfectly fine locally For context This was the error i was facing yesterdayI'm not sure why this works now lol 🥲 Maybe we were just one workflow re-run away from figuring out it was a temporary download failure yesterday. |
|
I looked into the issue and was able to reproduce both problems. The first one: # golang.org/x/tools/internal/tokeninternal
../../../go/pkg/mod/golang.org/x/tools@v0.20.0/internal/tokeninternal/tokeninternal.go:64:9: invalid array length -delta * delta (constant -256 of type int64)
make: *** [Makefile:208: controller-gen] Error 1This happens due to an incompatibility between If you run make test with Go 1.24.4, the
This occurs because looks like the bucket where older versions of the envtest binaries were stored has changed, and the required permissions are no longer available. Therefore, bumping the envtest version appears to be the correct fix. For now, I suggest keeping the version bump limited to envtest. The operator still uses Go 1.24.4, and |
Signed-off-by: karthik balasubramanian <karthikbalasubramanian08@gmail.com>
85df758 to
117d852
Compare
IrvingMg
left a comment
There was a problem hiding this comment.
Thanks for the fix, @kaizakin!
/lgtm
It looks like #279 is a broader PR that bumps several other dependencies, including envtest. However, we might want to start by bumping envtest first, since the version currently used in the repo seems to be broken and requires a fix.
Let’s see what @hasanawad94 thinks, as he’s working on the dependency bumps in #279.
adambkaplan
left a comment
There was a problem hiding this comment.
/approve
@hasanawad94 FYI - I'm going to ✅ this pull request so that we unblock CI testing. You will need to rebase your PR and drop your envtest change when the inevitable merge conflict arrives.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: adambkaplan The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Changes
Fixes #280
There are some autogenerated changes with this PR which im not sure about. :)
/kind cleanup
Submitter Checklist
See the contributor guide
for details on coding conventions, github and prow interactions, and the code review process.
Release Notes