Upgrade K8s libraries to v0.26.4 #4935

heanlan · 2023-05-04T22:07:06Z

k8s.io/apimachinery/pkg/util/sets.String was deprecated, and replaced with
sets.Set[string]. sets.String.List() was replaced with sets.List(sets.Set[string]).
Similar changes happened on sets.Int32, sets.Int64
k8s.io/apiserver/pkg/registry/rest.Storage interface added requirement on
implementation of Destroy() method
k8s.io/component-base/metrics/prometheus/ratelimiter was removed as it was
not doing anything. The call in third_party/ipam/nodeipam was removed
accordingly. Refer to PR: remove rate limiter metric as it is not in use kubernetes/kubernetes#113054
upgraded ginkgo to v2 as required by sigs.k8s.io/controller-runtime@v0.14.6.
timeout was removed from BeforeSuite, and was refactored referencing to
Timeout on setup nodes in ginkgo v2 onsi/ginkgo#882
k8s.io/client-go/tools/remotecommand.Executor.Stream was deprecated, and
replaced with StreamWithContext

Related to #4901

Signed-off-by: heanlan hanlan@vmware.com

antoninbas

Overall LGTM.

I added @tnqn and @luolanzone for extra reviews.

It would have been nice to keep the sets change in a separate commit, as it would have made it easier to focus the review process on the other changes, which require more attention.

I have pushed your new codegen image (antrea/codegen:kubernetes-1.26.4) to the Antrea Docker registry, after building it locally.

BTW, I thought you were going to include a fix for hack/make-metric-docs.sh?

antoninbas · 2023-05-16T21:28:47Z

multicluster/test/integration/suite_test.go

+	done := make(chan interface{})
+	go func() {
+		defer GinkgoRecover()
+		cfg, err = testEnv.Start()
+		close(done)
+	}()
+	Eventually(done).WithTimeout(1 * time.Minute).Should(BeClosed())


Is this based on some official Ginkgo examples?

yes, kind of? I found it in one of their issue, not in official documentation. I put the link of the issue in the PR description. Here is the exact example given by Ginkgo: onsi/ginkgo#882 (comment)

pkg/apiserver/registry/controlplane/egressgroup/rest.go

pkg/apiserver/registry/system/supportbundle/rest.go

antoninbas · 2023-05-16T21:47:34Z

test/e2e/framework.go

@@ -1957,8 +1957,10 @@ func (data *TestData) RunCommandFromPod(podNamespace string, podName string, con
 	if err != nil {
 		return "", "", err
 	}
+	ctx, cancelFn := context.WithTimeout(context.Background(), 1*time.Minute)


does that 1 minute timeout match the previous behavior of Stream?

I think so. The context is just for avoiding resource leak for some bad streams which will hang there forever. I thought the "longest" command we have in e2e tests is the one in flowaggregator_test, creating iperf traffic for 12s.

Ok, let's go with this then. We can add a context.Context parameter to the function later if needed

heanlan · 2023-05-16T22:11:22Z

BTW, I thought you were going to include a fix for hack/make-metric-docs.sh?

I met some issues. e.g. we use fmt cmd with -s option in the script https://github.com/antrea-io/antrea/blob/94fe42d683fd08d91ba519c0dd8a682e2031e3c0/hack/make-metrics-doc.sh#LL142C9-L142C60, which is for not joining lines. But I didn't find the similar option available for macOS. I didn't find easy alternatives, neither, so I gave up to do it in this PR.

linux: https://ss64.com/bash/fmt.html
macOS: https://ss64.com/osx/fmt.html

I doubt whether the script was ever working on macOS. The sed and fmt commands both need to be fixed. Probably we should reopen issue #3787 to track this?

antoninbas · 2023-05-18T02:12:58Z

build/images/codegen/Dockerfile

+# workaround for safe directory issue on github actions
+# ref: https://github.com/actions/runner-images/issues/6775
+RUN git config --global --add safe.directory /go/src/antrea.io/antrea


I'm fine with the solution overall, but I think we can make it a bit nice:

Avoid hardcoding the /go/src/antrea.io/antrea in the Docker file. Instead of using a RUN instruction in the Dockefile, let's update the update-codegen.sh script so that it looks like this:

set -o errexit set -o pipefail ANTREA_ROOT="$( cd "$( dirname "${BASH_SOURCE[0]}" )/../" && pwd )" IMAGE_NAME="antrea/codegen:kubernetes-1.26.4" function docker_run() { docker pull ${IMAGE_NAME} set -x ANTREA_PATH="/go/src/antrea.io/antrea" docker run --rm \ -e GOPROXY=${GOPROXY} \ -w ${ANTREA_PATH} \ -v ${ANTREA_ROOT}:${ANTREA_PATH} \ "${IMAGE_NAME}" bash -c "git config --global --add safe.directory ${ANTREA_PATH} && $@" } docker_run hack/update-codegen-dockerized.sh "$@"

Add a better comment in update-codegen.sh to explain why this is needed. Pointing to that issue is a bit misleading as this is orthogonal to Github actions. Maybe something like this:

Recent versions of Git will not access .git directories which are owned by another user (as a security measure), unless the directories are explicitly added to a "safe" list in the Git config. When we run the Docker container, the Antrea source directory may be owned (depends on the Docker platform) by a user which is different from the container user (as the source directory is mounted from the host). If this is the case, the Git program inside the container will refuse to run. This is why we explicitly add the Antrea source directory to the list of "safe" directories. We are still looking into the possibility of running the Docker container as the "current host user".

Thanks for the suggestions. Done.

* k8s.io/apimachinery/pkg/util/sets.String was deprecated, and replaced with sets.Set[string]. sets.String.List() was replaced with sets.List(sets.Set[string]). Similar changes happened on sets.Int32, sets.Int64 * k8s.io/apiserver/pkg/registry/rest.Storage interface added requirement on implementation of Destroy() method * k8s.io/component-base/metrics/prometheus/ratelimiter was removed as it was not doing anything. The call in third_party/ipam/nodeipam was removed accordingly. Refer to PR: kubernetes/kubernetes#113054 * upgraded ginkgo to v2 as required by sigs.k8s.io/controller-runtime@v0.14.6. timeout was removed from BeforeSuite, and was refactored referencing to onsi/ginkgo#882 * k8s.io/client-go/tools/remotecommand.Executor.Stream was deprecated, and replaced with StreamWithContext Related to antrea-io#4901 Signed-off-by: heanlan <hanlan@vmware.com>

Signed-off-by: heanlan <hanlan@vmware.com>

heanlan · 2023-05-18T20:15:10Z

/test-all

antoninbas · 2023-05-18T23:22:45Z

@heanlan please open a new issue for hack/make-metric-docs.sh on macOS, don't re-open the old one

antoninbas

LGTM

heanlan · 2023-05-18T23:31:54Z

Thanks for the review @antoninbas . Is it ok for us to merge this PR before Antrea 1.12? or we want to do it after the release

antoninbas · 2023-05-18T23:57:46Z

Thanks for the review @antoninbas . Is it ok for us to merge this PR before Antrea 1.12? or we want to do it after the release

I think we should be able to merge it, but I will let @tnqn make that call

tnqn

LGTM, thanks for the work.

* k8s.io/apimachinery/pkg/util/sets.String was deprecated, and replaced with sets.Set[string]. sets.String.List() was replaced with sets.List(sets.Set[string]). Similar changes happened on sets.Int32, sets.Int64 * k8s.io/apiserver/pkg/registry/rest.Storage interface added requirement on implementation of Destroy() method * k8s.io/component-base/metrics/prometheus/ratelimiter was removed as it was not doing anything. The call in third_party/ipam/nodeipam was removed accordingly. Refer to PR: kubernetes/kubernetes#113054 * upgraded ginkgo to v2 as required by sigs.k8s.io/controller-runtime@v0.14.6. timeout was removed from BeforeSuite, and was refactored referencing to onsi/ginkgo#882 * k8s.io/client-go/tools/remotecommand.Executor.Stream was deprecated, and replaced with StreamWithContext * make codegen and manifests * allow access from container users to git directories Related to antrea-io#4901 Signed-off-by: heanlan <hanlan@vmware.com>

heanlan marked this pull request as draft May 4, 2023 22:37

heanlan force-pushed the upgrade-k8s-libraries-to-v0.26.4 branch from fa0b7c9 to 3131280 Compare May 5, 2023 19:17

heanlan marked this pull request as ready for review May 5, 2023 20:14

heanlan force-pushed the upgrade-k8s-libraries-to-v0.26.4 branch from 3131280 to 9cc15d4 Compare May 16, 2023 20:02

heanlan requested a review from antoninbas May 16, 2023 20:48

antoninbas requested review from tnqn and luolanzone May 16, 2023 21:23

antoninbas reviewed May 16, 2023

View reviewed changes

heanlan force-pushed the upgrade-k8s-libraries-to-v0.26.4 branch from 012bf26 to feaad50 Compare May 17, 2023 23:36

antoninbas reviewed May 18, 2023

View reviewed changes

heanlan force-pushed the upgrade-k8s-libraries-to-v0.26.4 branch 2 times, most recently from 40e7d4d to ec550f0 Compare May 18, 2023 19:13

heanlan added 3 commits May 18, 2023 15:36

Make codegen and manifests

d4e5861

Signed-off-by: heanlan <hanlan@vmware.com>

Allow access from container users to git directories

1ec6ac7

Signed-off-by: heanlan <hanlan@vmware.com>

heanlan force-pushed the upgrade-k8s-libraries-to-v0.26.4 branch from ec550f0 to 1ec6ac7 Compare May 18, 2023 19:43

heanlan added the area/dependency Issues or PRs related to dependency changes. label May 18, 2023

antoninbas approved these changes May 18, 2023

View reviewed changes

tnqn approved these changes May 19, 2023

View reviewed changes

tnqn merged commit f3394c5 into antrea-io:main May 19, 2023

wenyingd mentioned this pull request May 19, 2023

Fix concurrent map write in mc controller #4994

Merged

heanlan deleted the upgrade-k8s-libraries-to-v0.26.4 branch May 19, 2023 17:28

heanlan mentioned this pull request Aug 8, 2023

upgrade antrea to v1.13.0 antrea-io/theia#413

Merged

luolanzone mentioned this pull request Jan 3, 2024

Compatibility with K8s 1.29 #5792

Closed

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade K8s libraries to v0.26.4 #4935

Upgrade K8s libraries to v0.26.4 #4935

heanlan commented May 4, 2023 •

edited

Loading

antoninbas left a comment

antoninbas May 16, 2023

heanlan May 16, 2023

antoninbas May 16, 2023

heanlan May 16, 2023 •

edited

Loading

antoninbas May 18, 2023

heanlan commented May 16, 2023 •

edited

Loading

antoninbas May 18, 2023

heanlan May 18, 2023

heanlan commented May 18, 2023

antoninbas commented May 18, 2023

antoninbas left a comment

heanlan commented May 18, 2023

antoninbas commented May 18, 2023

tnqn left a comment

Upgrade K8s libraries to v0.26.4 #4935

Upgrade K8s libraries to v0.26.4 #4935

Conversation

heanlan commented May 4, 2023 • edited Loading

antoninbas left a comment

Choose a reason for hiding this comment

antoninbas May 16, 2023

Choose a reason for hiding this comment

heanlan May 16, 2023

Choose a reason for hiding this comment

antoninbas May 16, 2023

Choose a reason for hiding this comment

heanlan May 16, 2023 • edited Loading

Choose a reason for hiding this comment

antoninbas May 18, 2023

Choose a reason for hiding this comment

heanlan commented May 16, 2023 • edited Loading

antoninbas May 18, 2023

Choose a reason for hiding this comment

heanlan May 18, 2023

Choose a reason for hiding this comment

heanlan commented May 18, 2023

antoninbas commented May 18, 2023

antoninbas left a comment

Choose a reason for hiding this comment

heanlan commented May 18, 2023

antoninbas commented May 18, 2023

tnqn left a comment

Choose a reason for hiding this comment

heanlan commented May 4, 2023 •

edited

Loading

heanlan May 16, 2023 •

edited

Loading

heanlan commented May 16, 2023 •

edited

Loading