-
Notifications
You must be signed in to change notification settings - Fork 370
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
Upgrade k8s libraries to v0.29.2 #5843
Conversation
hjiajing
commented
Jan 4, 2024
•
edited
Loading
edited
- Upgrade k8s libraries to v0.29.2.
- Upgrade controller-runtime to v0.16.3
Hi @tnqn , Could you please help to view this PR. It upgrades k8s libraries from 0.26.4 to 0.29.0. Major changes are about the function And the |
/test-multicluster-e2e |
build/images/codegen/Dockerfile
Outdated
@@ -32,11 +32,11 @@ LABEL description="A Docker image based on the golang image, which includes code | |||
|
|||
ENV GO111MODULE=on | |||
|
|||
ARG K8S_VERSION=1.26.4 | |||
ARG K8S_VERSION=1.29.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please search 1.26.4
in antrea repo, I noticed there a few other files need to be updated.
For new version of codegen image, please refer to build/images/codegen/README.md to build a new image, we need @tnqn or @antoninbas's help to upload it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated codegen's README and hack/update-codegen.sh
from v1.26.4
to v1.29.0
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
make sure to change this to 1.29.2
and update the README
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DONE. Replace v0.26.4
and v0.29.0
to 0.29.2
.
@@ -42,7 +44,6 @@ import ( | |||
antreamcscheme "antrea.io/antrea/multicluster/pkg/client/clientset/versioned/scheme" | |||
antreascheme "antrea.io/antrea/pkg/client/clientset/versioned/scheme" | |||
"antrea.io/antrea/pkg/signals" | |||
//+kubebuilder:scaffold:imports |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why to move this line?
I think it was generated automatically before, please help to check if possible to remove it, or it has a specific purpose. Thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment is the anchor point for kubebuilder
to insert code. And I did not remove it, it was formatted and moved in front of these lines. I have moved it back.
@@ -184,7 +184,7 @@ func (s *StretchedNetworkPolicyController) processNextWorkItem() bool { | |||
|
|||
if podRef, ok := obj.(types.NamespacedName); !ok { | |||
s.queue.Forget(obj) | |||
klog.Errorf("Expected type 'NamespacedName' in work queue but got object", "object", obj) | |||
klog.Errorf("Expected type 'NamespacedName' in work queue but got object %v\n", obj) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to append "\n", klog.Errorf will handle it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use structured logging (klog.ErrorS
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
d61dc7b
to
fd8e53b
Compare
141f6e9
to
a4678e1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LTGM overall, two nits.
cmd/antrea-agent/agent.go
Outdated
@@ -744,10 +744,10 @@ func run(o *Options) error { | |||
// Service would fail. | |||
if o.config.AntreaProxy.ProxyAll { | |||
klog.InfoS("Waiting for AntreaProxy to be ready") | |||
if err := wait.PollUntil(time.Second, func() (bool, error) { | |||
if err := wait.PollUntilContextCancel(ctx, time.Second, true, func(ctx context.Context) (bool, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this "immediate" be true for PollUtil?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same question
let's keep the same behavior
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it should be set to false
, I missed this one. Fixed.
/test-multicluster-e2e |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Out of curiosity, what are the nolint:staticcheck
pragmas for?
For contexts, I would follow this rule:
- use
context.Background()
when appropriate, in particular in unit tests - use
context.TODO()
when we may want to add an actual context object in the future (e.g., in the future maybe the function should take a context as parameter)
In general, I don't think we should have context.Background()
in a pkg
function.
cmd/antrea-agent/agent.go
Outdated
@@ -744,10 +744,10 @@ func run(o *Options) error { | |||
// Service would fail. | |||
if o.config.AntreaProxy.ProxyAll { | |||
klog.InfoS("Waiting for AntreaProxy to be ready") | |||
if err := wait.PollUntil(time.Second, func() (bool, error) { | |||
if err := wait.PollUntilContextCancel(ctx, time.Second, true, func(ctx context.Context) (bool, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same question
let's keep the same behavior
if o.SelfSignedCert { | ||
o.options.CertDir = selfSignedCertDir | ||
o.options.Metrics.CertDir = selfSignedCertDir | ||
} else { | ||
o.options.CertDir = certDir | ||
o.options.Metrics.CertDir = certDir | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure that's correct. The historic CertDir
option seems to have been used for both the metrics server and for the webhook server. Do we need to configure WebhookServer.CertDir
as well for the manager?
cc @luolanzone
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes @antoninbas is right, the historic CertDir is for webhook according to the comment. @hjiajing please help to double check and refine this part with correct fields. Thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Used webhook.NewServer
to initialize the webhook server with certDir.
@@ -256,13 +257,13 @@ func TestClusterSetMapFunc_Gateway(t *testing.T) { | |||
} | |||
fakeClient := fake.NewClientBuilder().WithScheme(common.TestScheme).WithObjects(clusterSet, gw1).Build() | |||
r := NewGatewayReconciler(fakeClient, common.TestScheme, "default", []string{"10.200.1.1/16"}, nil) | |||
requests := r.clusterSetMapFunc(clusterSet) | |||
requests := r.clusterSetMapFunc(context.TODO(), clusterSet) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for unit tests, I usually suggest defining ctx := context.Background()
at the beginning of the test function, then using ctx
throughout the test definition
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed, as well as other 3 files.
pkg/agent/agent.go
Outdated
} | ||
return true, nil | ||
}) | ||
err = wait.PollUntilContextTimeout(context.Background(), 200*time.Millisecond, 10*time.Second, true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe context.TODO()
here?
same for other instances in this file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done, changed to context.TODO()
.
pkg/agent/agent_linux.go
Outdated
klog.InfoS("OVS bridge local port is ready", "type", link.Type(), "attrs", link.Attrs()) | ||
return true, nil | ||
}) | ||
wait.PollUntilContextTimeout(context.Background(), 100*time.Millisecond, 10000*time.Millisecond, true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto
currentEP, ok := ic.getEndpoint(endpointName) | ||
if !ok { | ||
klog.InfoS("HNSEndpoint doesn't exist in cache, exit current goroutine", "HNSEndpoint", endpointName) | ||
pollErr := wait.PollUntilContextTimeout(context.Background(), 100*time.Millisecond, 60*time.Second, true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto
@@ -1118,7 +1118,7 @@ func TestSyncEgress(t *testing.T) { | |||
if tt.newLocalIPs != nil { | |||
c.localIPDetector = &fakeLocalIPDetector{localIPs: tt.newLocalIPs} | |||
} | |||
assert.NoError(t, wait.Poll(time.Millisecond*100, time.Second, func() (done bool, err error) { | |||
assert.NoError(t, wait.PollUntilContextTimeout(context.Background(), time.Millisecond*100, time.Second, false, func(ctx context.Context) (done bool, err error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this may be a good opportunity to update these to assert.Eventually
/ require.Eventually
what do you think @tnqn ?
Taking into account of course this shortcoming: stretchr/testify#1396
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
makes sense to me
@@ -184,7 +184,7 @@ func (s *StretchedNetworkPolicyController) processNextWorkItem() bool { | |||
|
|||
if podRef, ok := obj.(types.NamespacedName); !ok { | |||
s.queue.Forget(obj) | |||
klog.Errorf("Expected type 'NamespacedName' in work queue but got object", "object", obj) | |||
klog.Errorf("Expected type 'NamespacedName' in work queue but got object %v\n", obj) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use structured logging (klog.ErrorS
)
a34764f
to
35fbc93
Compare
@hjiajing did you answer this anywhere? |
@@ -246,7 +247,7 @@ func TestNamespaceMapFunc(t *testing.T) { | |||
mcReconciler.SetRemoteCommonArea(commonArea) | |||
|
|||
r := NewLabelIdentityReconciler(fakeClient, common.TestScheme, mcReconciler, "default") | |||
actualReq := r.namespaceMapFunc(ns) | |||
actualReq := r.namespaceMapFunc(context.TODO(), ns) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use context.Background()
like in multicluster/controllers/multicluster/member/gateway_controller_test.go?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done, changed all context.TODO()
in unit tests to context.Backgrond()
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure this was addressed. I still see context.TODO()
in this file
@@ -376,7 +377,7 @@ func TestAntreaIPAMDriver(t *testing.T) { | |||
|
|||
podNamespace := string(k8sArgsMap[test].K8S_POD_NAMESPACE) | |||
podName := string(k8sArgsMap[test].K8S_POD_NAME) | |||
err = wait.Poll(time.Millisecond*200, time.Second, func() (bool, error) { | |||
err = wait.PollUntilContextTimeout(context.TODO(), time.Millisecond*200, time.Second, false, func(ctx context.Context) (bool, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
context.Background()
in unit tests
@@ -1118,7 +1118,7 @@ func TestSyncEgress(t *testing.T) { | |||
if tt.newLocalIPs != nil { | |||
c.localIPDetector = &fakeLocalIPDetector{localIPs: tt.newLocalIPs} | |||
} | |||
assert.NoError(t, wait.Poll(time.Millisecond*100, time.Second, func() (done bool, err error) { | |||
assert.NoError(t, wait.PollUntilContextTimeout(context.TODO(), time.Millisecond*100, time.Second, false, func(ctx context.Context) (done bool, err error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
did you see my earlier comment: #5843 (comment)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry I forget to reply it, I saw it and I'm working on this part. Thanks for the reminder.
35fbc93
to
0895fe5
Compare
Because the |
I see the following recommendation in the package documentation:
Do we have a plan to migrate to our own config implementation? If it is straightforward, should we do it as part of this PR? cc @luolanzone |
Hi @antoninbas, we have one spec |
f400885
to
2320478
Compare
437374c
to
e26509e
Compare
Hi @antoninbas , I have changed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the delay.
I think we should update the PR to use 0.29.2 now, even if that means pushing a new container image for codegen. There is also a merge conflict.
Let's try to get this PR merged soon
if *multiclusterConfig.LeaderElection.LeaderElect { | ||
o.options.LeaderElection = true | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just took a quick look and it seems there are as lot of fields in multiclusterConfig.LeaderElection
besides LeaderElection
. I guess we are just ignoring these fields? Do we even need / want to support leader election here? I assume it's not something we leverage in the installation manifests.
Should we just include the fields we actually support in MultiClusterConfig
?
Alternatively, is there a standard way of converting from configv1alpha1.LeaderElectionConfiguration
to manager options? I took a look at the following PR, and it seems that they are copying fields one by one:
https://github.com/kubernetes-sigs/kueue/pull/889/files
cc @luolanzone
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We actually didn't use LeaderElection in MC. I think it's stale codes which can be removed. @hjiajing could you help to remove it? Please also remove the leaderElection
in the manifests, Thanks.
@@ -246,7 +247,7 @@ func TestNamespaceMapFunc(t *testing.T) { | |||
mcReconciler.SetRemoteCommonArea(commonArea) | |||
|
|||
r := NewLabelIdentityReconciler(fakeClient, common.TestScheme, mcReconciler, "default") | |||
actualReq := r.namespaceMapFunc(ns) | |||
actualReq := r.namespaceMapFunc(context.TODO(), ns) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure this was addressed. I still see context.TODO()
in this file
require.NoError(t, wait.PollImmediate(10*time.Millisecond, time.Second, func() (done bool, err error) { | ||
return c.queue.Len() == 1, nil | ||
})) | ||
require.NoError(t, wait.PollUntilContextTimeout(context.Background(), 10*time.Millisecond, time.Second, true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe require.Eventually
then, given that you have made the change for assert
?
same comment in other places as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure this was addressed. I still see context.TODO() in this file
Sorry I missed this file. Changed to context.Background()
. And all require.NoError(t, wait.XXX)
were changed to require.Eventually
.
err := wait.PollImmediate(10*time.Millisecond, 100*time.Millisecond, func() (bool, error) { | ||
return ruleHasBeenDeleted(), nil | ||
}) | ||
err := wait.PollUntilContextTimeout(context.Background(), 10*time.Millisecond, 100*time.Millisecond, true, | ||
func(ctx context.Context) (bool, error) { | ||
return ruleHasBeenDeleted(), nil | ||
}) | ||
require.Error(t, err, "Rule ID was unexpectedly released") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same comment: could use require.Eventually
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like you didn't push the code? I still see wait.PollUntilContextTimeout
+ require.Error
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry I missed this file, fixed. And I search globally, this file is the only one which uses wait.PollUntilContextTimeout
+ require.Error
. Replaced this with require.Eventually
.
err = wait.PollUntilContextTimeout(context.Background(), 10*time.Millisecond, 1*time.Second, true, | ||
func(ctx context.Context) (bool, error) { | ||
return ruleHasBeenDeleted(), nil | ||
}) | ||
require.NoError(t, err, "Rule ID was not released") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same thing here, I haven't checked the rest of the code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
@@ -199,7 +200,7 @@ func TestApplyServerCert(t *testing.T) { | |||
|
|||
if tt.selfSignedCert && tt.testRotate { | |||
oldCertKeyContent := got.getCertificate() | |||
err := wait.Poll(time.Second, 8*time.Second, func() (bool, error) { | |||
err := wait.PollUntilContextTimeout(context.Background(), time.Second, 8*time.Second, false, func(ctx context.Context) (bool, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use assert.Eventually
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
@@ -308,6 +308,10 @@ func (r *supportBundleREST) clean(ctx context.Context, bundlePath string, durati | |||
defaultFS.Remove(bundlePath) | |||
} | |||
|
|||
func (r *supportBundleREST) GetSingularName() string { | |||
return "supportBundle" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unless I am mistaken, this should be "supportbundle"
, not "supportBundle"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
test/integration/ovs/ofctrl_test.go
Outdated
require.NoError(t, wait.PollUntilContextTimeout(context.Background(), time.Millisecond*100, time.Second, true, | ||
func(ctx context.Context) (done bool, err error) { | ||
flowList, err = OfctlDumpTableFlowsWithoutName(ovsCtlClient, myTable.GetID()) | ||
require.Nil(t, err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not from this PR, but should be require.NoError
same for other tests below
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed all require.Nil(t, err)
to require.NoError(t, err)
d6073af
to
ccebda2
Compare
OK, all feature gates enabled E2E failed, I'm triaging this. |
@hjiajing that failure may be unrelated to your PR. However, the |
Sure, I will regenerate this doc. |
ccebda2
to
2822cac
Compare
Doc updated. |
ae08465
to
8bbe7b0
Compare
pkg/apiserver/registry/networkpolicy/clustergroupmember/rest.go
Outdated
Show resolved
Hide resolved
1. Upgrade k8s libraries to v0.29.2 2. Upgrade controller-runtime to v0.16.3 Signed-off-by: hjiajing <[email protected]>
8bbe7b0
to
77a1eb9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
/test-all |
Fix antrea-io#6129 In the failure tests, the following function is called to verify whether a connection should be allowed or denied. To verify a connection should be denied, it requires 5 seconds. ``` func probeClientIPFromPod(data *TestData, pod, container string, baseUrl string) (string, error) { url := fmt.Sprintf("%s/%s", baseUrl, "clientip") hostPort, _, err := data.runWgetCommandFromTestPodWithRetry(pod, data.testNamespace, container, url, 5) if err != nil { return "", err } host, _, err := net.SplitHostPort(hostPort) return host, err } ``` Before antrea-io#5843, these e2e tests utilized the function PollImmediate from k8s.io/apimachinery/pkg/util/wait, which immediately calls an anonymous function including the above function. Since the timeout is 5 seconds, and the ticker time is 1 second, and the anonymous function runs immediately, the 5-second timeout is sufficient to verify the denied state of a connection as mentioned above. However, after antrea-io#5843, the function `Eventually` from github.com/stretchr/testify/assert is used with the same parameters, which implies that the anonymous function runs after the first ticker time, leaving 4 seconds. 4 seconds are insufficient to verify the denied state of a connection. To resolve the issue, the timeout should be adjusted to be more than 5 seconds. Signed-off-by: Hongliang Liu <[email protected]>
Fix antrea-io#6129 In the failure tests, the following function is called to verify whether a connection should be allowed or denied. To verify a connection should be denied, it requires 5 seconds. ```go func probeClientIPFromPod(data *TestData, pod, container string, baseUrl string) (string, error) { url := fmt.Sprintf("%s/%s", baseUrl, "clientip") hostPort, _, err := data.runWgetCommandFromTestPodWithRetry(pod, data.testNamespace, container, url, 5) if err != nil { return "", err } host, _, err := net.SplitHostPort(hostPort) return host, err } ``` Before antrea-io#5843, these e2e tests utilized the function `PollImmediate` from `k8s.io/apimachinery/pkg/util/wait`, which immediately calls an anonymous function including the above function. Since the timeout is 5 seconds, and the ticker time is 1 second, and the anonymous function runs immediately, the 5-second timeout is sufficient to verify the denied state of a connection as mentioned above. However, after antrea-io#5843, the function `Eventually` from `github.com/stretchr/testify/assert` is used with the same parameters, which implies that the anonymous function runs after the first ticker time, leaving 4 seconds. 4 seconds are insufficient to verify the denied state of a connection. To resolve the issue, `RunCommandFromPod` called in `data.runWgetCommandFromTestPodWithRetry` is called directly in function `Eventually` to verify the connection state. Signed-off-by: Hongliang Liu <[email protected]>
Fix antrea-io#6129 In the failure tests, the following function is called to verify whether a connection should be allowed or denied. To verify a connection should be denied, it requires 5 seconds. ```go func probeClientIPFromPod(data *TestData, pod, container string, baseUrl string) (string, error) { url := fmt.Sprintf("%s/%s", baseUrl, "clientip") hostPort, _, err := data.runWgetCommandFromTestPodWithRetry(pod, data.testNamespace, container, url, 5) if err != nil { return "", err } host, _, err := net.SplitHostPort(hostPort) return host, err } ``` Before antrea-io#5843, these e2e tests utilized the function `PollImmediate` from `k8s.io/apimachinery/pkg/util/wait`, which immediately calls an anonymous function including the above function. Since the timeout is 5 seconds, and the ticker time is 1 second, and the anonymous function runs immediately, the 5-second timeout is sufficient to verify the denied state of a connection as mentioned above. However, after antrea-io#5843, the function `Eventually` from `github.com/stretchr/testify/assert` is used with the same parameters, which implies that the anonymous function runs after the first ticker time, leaving 4 seconds. 4 seconds are insufficient to verify the denied state of a connection. To resolve the issue, `RunCommandFromPod` called in `data.runWgetCommandFromTestPodWithRetry` is called directly in function `Eventually` to verify the connection state. Signed-off-by: Hongliang Liu <[email protected]>
After switching from `wait.PollImmediate` to `assert.Eventually` in #5843, the probe used to validate L7 NP enforcement was no longer correct. We improve the validation logic so that each iteration of the condition function in `assert.Eventually` only sends an HTTP probe once, instead of using a probe with its own retry mechanism. This fixes the issue. Fixes #6129 Signed-off-by: Hongliang Liu <[email protected]>
We re-apply antrea-io#5703. When updating K8s dependencies in antrea-io#5843, the change was reverted and we went back to using an older version of opentelemetry which is affected by a CVE. The CVE doesn't affect Antrea, the patch is meant to avoid warnings from CVE scanners. Co-authored-by: Antonin Bas <[email protected]> Signed-off-by: Bin Liu <[email protected]> Signed-off-by: Antonin Bas <[email protected]>
A few issues were introduced by antrea-io#5843 because of changes in the sigs.k8s.io/controller-runtime interface. The biggest issue was that the call to ctrl.NewManager was not using the Options object populated earlier in the setupManagerAndCertController function. Instead it was creating and using a new, incomplete Options object. Fixes antrea-io#6149 Signed-off-by: Antonin Bas <[email protected]>
A few issues were introduced by antrea-io#5843 because of changes in the sigs.k8s.io/controller-runtime interface. The biggest issue was that the call to ctrl.NewManager was not using the Options object populated earlier in the setupManagerAndCertController function. Instead it was creating and using a new, incomplete Options object. Fixes antrea-io#6149 Signed-off-by: Antonin Bas <[email protected]>
A few issues were introduced by antrea-io#5843 because of changes in the sigs.k8s.io/controller-runtime interface. The biggest issue was that the call to ctrl.NewManager was not using the Options object populated earlier in the setupManagerAndCertController function. Instead it was creating and using a new, incomplete Options object. Fixes antrea-io#6149 Signed-off-by: Antonin Bas <[email protected]>
A few issues were introduced by #5843 because of changes in the sigs.k8s.io/controller-runtime interface. The biggest issue was that the call to ctrl.NewManager was not using the Options object populated earlier in the setupManagerAndCertController function. Instead it was creating and using a new, incomplete Options object. Additionally, the decoder is no longer injected automatically, it needs to be instantiated by us. Otherwise the admission webhook panics. See kubernetes-sigs/controller-runtime#2695 Fixes #6149 Signed-off-by: Antonin Bas <[email protected]>
We re-apply #5703. When updating K8s dependencies in #5843, the change was reverted and we went back to using an older version of opentelemetry which is affected by a CVE. The CVE doesn't affect Antrea, the patch is meant to avoid warnings from CVE scanners. Signed-off-by: Bin Liu <[email protected]> Signed-off-by: Antonin Bas <[email protected]> Co-authored-by: Bin Liu <[email protected]>