Skip to content

Conversation

@pinniped-ci-bot
Copy link
Collaborator

Automatically bumped all go.mod direct dependencies and/or images in dockerfiles.

@joshuatcasey
Copy link
Contributor

Currently this is blocked because I upgraded CI to use go1.25 which ran into this issue: golang/go#74462

We can see the issue show up in CI here: https://ci.pinniped.broadcom.net/teams/main/pipelines/dockerfile-builders/jobs/build-k8s-code-generator-1.30/builds/56#L689eb8de:602

@cfryanr
Copy link
Contributor

cfryanr commented Aug 18, 2025

Note that the maintainers of x/tools say that they will fix the issue in x/tools@v0.24.1 and x/tools@v0.25.1. They also say that the issue is only with x/tools v0.8.0 through v0.25.0. But we're using v0.34.0.

@cfryanr
Copy link
Contributor

cfryanr commented Aug 18, 2025

Oh I see, the issue is not with Pinniped's production code, but is with building our codegen container image for Kube 1.30 - 1.33. These are trying to install tools which are using old versions of x/tools. It could fail while running any go install in the CI script https://github.com/vmware/pinniped/blob/ci/dockerfiles/k8s-code-generator/setup.sh.

The first go install is for k8s.io/code-generator:

  • k8s.io/code-generator v0.30.14 uses x/tools v0.18.0. This is a broken version of x/tools. Kubernetes 1.30 is not supported anymore, so I doubt they will fix this version.
  • k8s.io/code-generator v0.31.12 uses x/tools v0.21.1-0.20240508182429-e35e4ccd0d2d. This is a broken version of x/tools. Kubernetes 1.31 is still supported until 2025-10-28, so maybe they will upgrade x/tools in a patch release?
  • k8s.io/code-generator v0.32.8 uses x/tools v0.26.0. This should be okay.
  • k8s.io/code-generator v0.33.4 uses x/tools v0.26.0. This should be okay.

It could also fail while running go install for k8s.io/kube-openapi/cmd/openapi-gen. As of today, that project is using x/tools v0.24.0. This is a broken version of x/tools.

The script also runs go install for sigs.k8s.io/controller-tools/cmd/controller-gen. The latest version is using x/tools v0.32.0, so that should not be a problem.

The script also runs go install for github.com/elastic/crd-ref-docs. The latest version is using x/tools v0.35.0, so that should also not be a problem.

I confirmed that:

It looks like according to the same comment that I linked to above, they might fix Golang itself for this issue, but not until 1.25.1 estimated to be released in September.

One possible workaround would be for the CI script to clone the source code of these problematic projects locally, sed the go.mod files to replace the broken versions with the good versions, and then run go install using the local copy of the source code. Presumably upgrading x/tools for these projects would not break any functionality of these projects.

Another option might be to use older versions of Go only for those Kubernetes versions which don't build with Go 1.25. But that might impact the toolchain directives in the generated go.mod files, so that might not be good.

Another option would be to wait a few weeks to see if they decide to fix this in Go 1.25.1.

@cfryanr
Copy link
Contributor

cfryanr commented Aug 19, 2025

It seems like Go 1.25's boringcrypto might be linking the executable differently. The FIPS image build job in CI was failing and I had to make this change to make it pass: 4d23e8d

Is this going to cause this old bug to re-emerge? #1369

We should investigate before merging this PR.

@cfryanr
Copy link
Contributor

cfryanr commented Aug 20, 2025

Attempting a workaround for the above issue with old x/tools packages and Go 1.25.0 here: f8781b4

@cfryanr
Copy link
Contributor

cfryanr commented Aug 21, 2025

For some reason, Go 1.25.0 decides that it does not want to add the toolchain directive to our auto-generated go.mod files when we run go mod tidy inside hack/update.sh anymore, unlike previous versions of Go. I'm fine with removing the toolchain directive from those files, so I aded a commit to this PR and paused the CI pipeline that creates/updates these automatic PRs so it does not overwrite my commit on this PR.

@cfryanr
Copy link
Contributor

cfryanr commented Aug 21, 2025

It seems like Go 1.25's boringcrypto might be linking the executable differently. The FIPS image build job in CI was failing and I had to make this change to make it pass: 4d23e8d

Is this going to cause this old bug to re-emerge? #1369

We should investigate before merging this PR.

On my laptop, building this PR's branch, running docker run --entrypoint /usr/local/bin/pinniped-concierge-kube-cert-agent --rm <local_container_build_id> sleep, and then running docker stats in another window, I can see that the process is using about 2.812MiB of memory. Note that this is a non-FIPS arm64 build.

Same experiment on a linux amd64 server, and I can see that it is using 2.621MiB of memory. Using the container image from the Pinniped v0.40.0 release, I see that it uses 2.629MiB. v0.22.0 (the release where the memory bug was fixed) uses 2.656MiB. v0.21.0 (before the memory bug was fixed) uses 9.004MiB. These are all for non-FIPS builds. So the memory usage since the fix has not changed for non-FIPS builds.

Still on the same linux server, trying the same experiment again but now using docker build -f ./hack/Dockerfile_fips . to build with FIPS compatibility, the kube cert agent built from this PR branch uses 2.809MiB to sleep. Repeating the same experiment on the main branch (which today uses Go 1.24.5) shows that the kube cert agent uses 1.211MiB to sleep. So it seems that the memory usage has more than doubled for kube cert agent FIPS builds using Go 1.25.0. However, it is still less than 3MiB. In the Deployment manifest of the kube cert agent, we set the memory request and limit both at 32Mi. So at the resting state, the process is not even close to its memory limit. Hopefully this still leaves enough overhead to allow multiple Concierge pods to exec into this pod to run other kube cert agent subcommands.

@cfryanr
Copy link
Contributor

cfryanr commented Aug 21, 2025

For some reason I also had to make this change to our CI script to get the integration tests to run with Go 1.25.0. Not sure why we never needed this change before. f8781b4...b65f533

@cfryanr cfryanr enabled auto-merge August 21, 2025 21:39
@cfryanr cfryanr merged commit 95e4ec9 into main Aug 22, 2025
29 checks passed
@cfryanr cfryanr deleted the pinny/bump-deps branch August 22, 2025 16:17
@cfryanr cfryanr mentioned this pull request Aug 28, 2025
21 tasks
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.

4 participants