-
Notifications
You must be signed in to change notification settings - Fork 369
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
Add e2e test for the NodeLatencyMonitor feature #6612
Add e2e test for the NodeLatencyMonitor feature #6612
Conversation
947491a
to
d3c1f7b
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.
All e2e tests go under test/e2e
and are written in a specific way - you could say we have a "framework" that we use for all our e2e tests. Please refer to that folder for examples. There are a lot of test files you can look at, such as test/e2e/basic_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.
Refactored e2e Tests: Updated the e2e tests to align with the framework established in the test/e2e directory. I’ve restructured the tests to match the format used in test/e2e/basic_test.go, ensuring consistency across our test suite. These changes should address your feedback and improve alignment with our testing standards. Thanks for your guidance, @antoninbas .
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.
The proposed e2e tests are not actually testing anything. You are just validating CRUD operations on the NodeLatencyMonitor CRD. These CRUD operations are provided by K8s, not Antrea, so in essence you are testing CRD support in K8s, and not testing the NodeLatencyMonitor feature in Antrea.
The issue for this task (#6549) describes what operations the e2e tests should cover.
test/e2e/nodelatencymonitor_test.go
Outdated
// GetKubeconfigPath returns the path to the kubeconfig file used for connecting to the Kubernetes cluster. | ||
func GetKubeconfigPath() (string, error) { | ||
// Check if the KUBECONFIG environment variable is set | ||
kubeconfig := os.Getenv("KUBECONFIG") | ||
if kubeconfig == "" { | ||
// Default to the kubeconfig path in the user's home directory if not set | ||
kubeconfig = filepath.Join(os.Getenv("HOME"), ".kube", "config") | ||
} | ||
|
||
// Verify that the kubeconfig file exists | ||
if _, err := os.Stat(kubeconfig); os.IsNotExist(err) { | ||
return "", fmt.Errorf("kubeconfig file does not exist at path: %s", kubeconfig) | ||
} | ||
|
||
return kubeconfig, nil | ||
} | ||
|
||
// getAntreaClient returns a clientset for interacting with Antrea CRDs | ||
func getAntreaClient(t *testing.T) *clientset.Clientset { | ||
kubeconfig, err := GetKubeconfigPath() | ||
failOnError(err, t) | ||
|
||
config, err := clientcmd.BuildConfigFromFlags("", kubeconfig) | ||
failOnError(err, t) | ||
|
||
antreaClientset, err := clientset.NewForConfig(config) | ||
failOnError(err, t) | ||
return antreaClientset | ||
} |
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 refer to other e2e tests. The framework already provides access to clientsets for K8s / Antrea, and there is no need for these 2 functions.
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.
okk, I understand. I'm doiing.
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.
Hii @antoninbas i'm so close towards my goal but
getting this error in test file
nodelatencymonitor_test.go:143: Failed to retrieve NodeLatencyStats: the server could not find the requested resource (get nodelatencystats.crd.antrea.io)
also
kubectl get nodelatencystats
error: the server doesn't have a resource type "nodelatencystats"
is there any thing i have missed?
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.
nodelatencystats.crd.antrea.io
does not exist as NodeLatencyStats
is not a CRD
kubectl get nodelatencystats
should work fine unless you are using an older version of Antrea (prior to v2.1). You should be testing against the "latest" Antrea (main branch).
When running kubectl api-resources
, you should see the following:
...
antreaclusternetworkpolicystats stats.antrea.io/v1alpha1 false AntreaClusterNetworkPolicyStats
antreanetworkpolicystats stats.antrea.io/v1alpha1 true AntreaNetworkPolicyStats
multicastgroups stats.antrea.io/v1alpha1 false MulticastGroup
networkpolicystats stats.antrea.io/v1alpha1 true NetworkPolicyStats
nodelatencystats stats.antrea.io/v1alpha1 false NodeLatencyStats
...
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.
@antoninbas following the previous feedback, i have refactored the e2e test for nodelatencymonitor
feature to focus on functional testing of Antrea .
- Validate the correct enabling and disabling of latency probes.
- Ensure the PingIntervalSeconds is respected and updated as expected.
- Check that latency stats are accurately recorded and retrieved, aligning with the intended behavior of the NodeLatencyMonitor feature in Antrea.
I think it will meet the necessary requirements.
test/e2e/nodelatencymonitor_test.go
Outdated
nlm := &v1alpha1.NodeLatencyMonitor{ | ||
ObjectMeta: v1.ObjectMeta{ | ||
Name: nodeLatencyMonitorName, | ||
Namespace: nodeLatencyMonitorNS, |
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.
It's a cluster-scoped CRD, there should be no namespace
test/e2e/nodelatencymonitor_test.go
Outdated
) | ||
|
||
func TestNodeLatencyMonitor(t *testing.T) { | ||
skipIfNotRequired(t) |
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 why you included this? With a single argument, I believe this is a no-op
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.
however, the test should be skipped if the feature (NodeLatencyMonitor
) is disabled. We have the skipIfFeatureDisabled
helper function for this.
test/e2e/nodelatencymonitor_test.go
Outdated
|
||
data, err := setupTest(t) | ||
if err != nil { | ||
t.Fatalf("Failed to set up clients: %v", 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.
the error message doesn't seem accurate
test/e2e/nodelatencymonitor_test.go
Outdated
t.Fatalf("Failed to delete NodeLatencyMonitor CR: %v", err) | ||
} | ||
|
||
waitForLatencyProbesDisabled(t, data) |
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.
does this really serve a purpose? The resource has no finalizers, so it should be deleted immediately, and I think there is no point in querying the API again to check for deletion. Is that no what you observed?
test/e2e/nodelatencymonitor_test.go
Outdated
func testEnableLatencyProbes(t *testing.T, data *TestData) { | ||
waitForLatencyProbesEnabled(t, data) | ||
} |
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 what the point of this test is?
test/e2e/nodelatencymonitor_test.go
Outdated
t.Run("testEnableLatencyProbes", func(t *testing.T) { | ||
testEnableLatencyProbes(t, data) | ||
}) | ||
t.Run("testRetrieveLatencyStats", func(t *testing.T) { | ||
testRetrieveLatencyStats(t, data) | ||
}) | ||
t.Run("testUpdatePingInterval", func(t *testing.T) { | ||
testUpdatePingInterval(t, data) | ||
}) | ||
t.Run("testDisableLatencyProbes", func(t *testing.T) { | ||
testDisableLatencyProbes(t, data) | ||
}) |
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.
it's better if subtests are independent and don't assume a specific execution order
204c0cb
to
58f8ad0
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.
@antoninbas i have made your desire changes .
I've run the sub tests independently, they works fine.
test/e2e/nodelatencymonitor_test.go
Outdated
func generatePingInterval() int32 { | ||
return int32(rand.Intn(21) + 30) | ||
} |
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.
there is no reason to use a random value for the ping interval
test/e2e/nodelatencymonitor_test.go
Outdated
"github.com/stretchr/testify/assert" | ||
"github.com/stretchr/testify/require" | ||
"k8s.io/apimachinery/pkg/api/errors" | ||
v1 "k8s.io/apimachinery/pkg/apis/meta/v1" |
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.
Renaming the import to v1
here has no effect as it is already the default name. I suggest renaming it to metav1
instead, which is what we usually do.
test/e2e/nodelatencymonitor_test.go
Outdated
"testing" | ||
"time" | ||
|
||
v1alpha1 "antrea.io/antrea/pkg/apis/crd/v1alpha1" |
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.
Renaming the import to v1alpha1
here has no effect as it is already the default name. I suggest renaming it to crdv1alpha1
instead, which is what we usually do.
test/e2e/nodelatencymonitor_test.go
Outdated
} | ||
|
||
func TestNodeLatencyMonitor(t *testing.T) { | ||
|
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 for an empty line here
test/e2e/nodelatencymonitor_test.go
Outdated
|
||
ctx := context.Background() | ||
|
||
_, err := data.crdClient.StatsV1alpha1().NodeLatencyStats().Create(ctx, summary, v1.CreateOptions{}) |
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.
there is a misunderstanding here. It is Antrea that should create the NodeLatencyStats
object as a result of enabling the NodeLatencyMonitor functionality. The test case should not be creating 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.
yes, i have some confusion here.
can you provide me documentation of 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.
Documentation for the feature is available here: https://github.com/antrea-io/antrea/blob/main/docs/feature-gates.md#nodelatencymonitor
The NodeLatencyMonitor CRD is to enable ICMP probes, which will measure latency between pairs of Nodes. The latency measurements are then reported by Antrea through the NodeLatencyStats API. The e2e test should validate that when enabling NodeLatencyMonitor, invoking the NodeLatencyStats API (GET) returns appropriate measurements.
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.
One thing to clarify,
My implementation is almost right other than the nodeLatencyStats creation part?
Or the whole function is wrong ?
test/e2e/nodelatencymonitor_test.go
Outdated
} | ||
} | ||
|
||
func testEnableLatencyProbes(t *testing.T, data *TestData) { |
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 test does not do anything. It just creates the CR (which only involves the K8s API), and it does not check that NodeLatencyStats
are reported correctly.
I would recommend having a single test case in this PR, which will
- create the CR, with a ping interval of 10s
- poll until
NodeLatencyStats
are reported correctly - delete the CR
We can improve testing over time.
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.
so, as the flow of test case
-
create the CR, with a ping interval of 10s
-
delete the CR
this are implemented -
poll until NodeLatencyStats are reported correctly
functionality i have do to correction.
update ping Interval will be deleted from here..
right?
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.
You have inverted steps 2 and 3
update ping Interval will be deleted from here..
yes, and you should only have one test, not 4
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.
Okk
3e765c9
to
321dbdc
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.
hi @antoninbas , this changes in test surely match with requirements.
test/e2e/nodelatencymonitor_test.go
Outdated
t.Logf("NodeLatencyMonitor CR created successfully.") | ||
|
||
// 2: Poll until(5min) NodeLatencyStats are reported correctly | ||
ctx := context.Background() |
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 use context.Background()
here when you use context.TODO()
for other API calls?
test/e2e/nodelatencymonitor_test.go
Outdated
|
||
// 2: Poll until(5min) NodeLatencyStats are reported correctly | ||
ctx := context.Background() | ||
err = wait.PollImmediate(time.Second, 300*time.Second, func() (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.
I am not sure what kind of checks you are doing in the condition function. You are computing max latency and average latency, but then you just log them? This is the test so it should validate the contents of the NodeLatencyStats
objects to make sure they are correct.
I would suggest the following:
- a first call to
wait.PollImmediate
, with a 30s timeout (after all the ping interval is set to 10s). This should validate that theNodeLatencyStats
objects are available. There should be one per Node, and for each one, there should be one entry inPeerNodeLatencyStats
for each Node. You can also make sure thatTargetIPLatencyStats
has the correct size (1 if cluster is IPv4-only or IPv6-only, 2 if cluster is dual-stack, and that each latency number is strictly positive. - a second call to
wait.PollImmediate
, again with a 30s timeout, which validates that the stats are refreshed after some time. You should perform the same validations as for the first call towait.PollImmediate
, but also check that all theLastSendTime
andLastRecvTime
fields have been updated.
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.
@antoninbas i've done the recommended changes.
test/e2e/nodelatencymonitor_test.go
Outdated
return false, err | ||
} | ||
|
||
if len(statsList.Items) == 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.
the length should match the number of Nodes in the cluster
test/e2e/nodelatencymonitor_test.go
Outdated
} | ||
|
||
for _, item := range statsList.Items { | ||
if len(item.PeerNodeLatencyStats) == 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.
ditto, the length should match the number of Nodes in the cluster
test/e2e/nodelatencymonitor_test.go
Outdated
return false, nil | ||
} | ||
for _, peerStat := range item.PeerNodeLatencyStats { | ||
if len(peerStat.TargetIPLatencyStats) == 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 read #6612 (comment) again, the expected length depends on the type of cluster
test/e2e/nodelatencymonitor_test.go
Outdated
} | ||
for _, targetStat := range peerStat.TargetIPLatencyStats { | ||
if targetStat.LastMeasuredRTTNanoseconds <= 0 { | ||
t.Logf("Invalid(negetive) RTT for peer %s on node %s", peerStat.NodeName, item.Name) |
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.
t.Logf("Invalid(negetive) RTT for peer %s on node %s", peerStat.NodeName, item.Name) | |
t.Logf("Invalid RTT for peer %s reported by Node %s", peerStat.NodeName, item.Name) | |
test/e2e/nodelatencymonitor_test.go
Outdated
previousSendTime = targetStat.LastSendTime | ||
previousRecvTime = targetStat.LastRecvTime |
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.
The "previous" values must come from the previous poll. We need a previous value for each pair of Node, so you will need a map for this.
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.
@antoninbas I've refine the nodelatencystats test.
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.
Overall the structure of the test looks good now, so all the new comments are suggestions for smaller improvements
test/e2e/nodelatencymonitor_test.go
Outdated
nodes, err := data.clientset.CoreV1().Nodes().List(context.TODO(), metav1.ListOptions{}) | ||
require.NoError(t, err, "Failed to list nodes") | ||
|
||
var isDualStack bool | ||
for _, node := range nodes.Items { | ||
if node.Spec.PodCIDR != "" && strings.Contains(node.Spec.PodCIDR, ":") { | ||
isDualStack = true | ||
break | ||
} | ||
} |
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.
you should be using clusterInfo.podV4NetworkCIDR
and clusterInfo.podV4NetworkCIDR
to determine this
we already have a framework that performs the correct checks for you
test/e2e/nodelatencymonitor_test.go
Outdated
t.Logf("NodeLatencyMonitor CR created successfully.") | ||
|
||
validateNodeLatencyStats := func(statsList *v1alpha1.NodeLatencyStatsList, initialPoll bool, previousTimes map[string]map[string]metav1.Time) (bool, error) { | ||
if len(statsList.Items) != len(nodes.Items) { |
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.
clusterInfo.numNodes
stores the number of Nodes, it is populated by the framework
test/e2e/nodelatencymonitor_test.go
Outdated
require.NoError(t, err, "Failed to create NodeLatencyMonitor CR") | ||
t.Logf("NodeLatencyMonitor CR created successfully.") | ||
|
||
validateNodeLatencyStats := func(statsList *v1alpha1.NodeLatencyStatsList, initialPoll bool, previousTimes map[string]map[string]metav1.Time) (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.
Move the previousTimes
map declaration before the closure definition, and remove the previousTimes
parameter
test/e2e/nodelatencymonitor_test.go
Outdated
|
||
for _, item := range statsList.Items { | ||
if len(item.PeerNodeLatencyStats) != len(nodes.Items)-1 { | ||
t.Logf("Expected %d PeerNodeLatencyStats for node %s, but found %d, retrying...", len(nodes.Items), item.Name, len(item.PeerNodeLatencyStats)) |
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.
invalid log message, the expected number of len(nodes.Items)-1
, not len(nodes.Items)
test/e2e/nodelatencymonitor_test.go
Outdated
|
||
for _, item := range statsList.Items { | ||
if len(item.PeerNodeLatencyStats) != len(nodes.Items)-1 { | ||
t.Logf("Expected %d PeerNodeLatencyStats for node %s, but found %d, retrying...", len(nodes.Items), item.Name, len(item.PeerNodeLatencyStats)) |
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.
nit: in Antrea we capitalize all K8s terms (such as Node
) in comments and log messages
test/e2e/nodelatencymonitor_test.go
Outdated
previousTimes[item.Name][peerStat.NodeName] = targetStat.LastSendTime | ||
} else { | ||
previousSendTime, sendTimeExists := previousTimes[item.Name][peerStat.NodeName] | ||
if !sendTimeExists || previousSendTime.Equal(&targetStat.LastSendTime) { |
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.
nit: for the check, I think it would be slightly better to validate that targetStat.LastSendTime > previousSendTime
test/e2e/nodelatencymonitor_test.go
Outdated
return false, err | ||
} | ||
|
||
if len(statsList.Items) == 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.
this seems redundant, given the checks already in validateNodeLatencyStats
, so maybe we could remove it?
test/e2e/nodelatencymonitor_test.go
Outdated
return false, err | ||
} | ||
|
||
if len(statsList.Items) == 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.
ditto
test/e2e/nodelatencymonitor_test.go
Outdated
err = wait.PollImmediate(time.Second, 30*time.Second, func() (bool, error) { | ||
statsList, err := data.crdClient.StatsV1alpha1().NodeLatencyStats().List(context.TODO(), metav1.ListOptions{}) | ||
if err != nil { | ||
t.Logf("Error while listing NodeLatencyStats: %v", 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.
no need for this log message
test/e2e/nodelatencymonitor_test.go
Outdated
err = wait.PollImmediate(time.Second, 30*time.Second, func() (bool, error) { | ||
statsList, err := data.crdClient.StatsV1alpha1().NodeLatencyStats().List(context.TODO(), metav1.ListOptions{}) | ||
if err != nil { | ||
t.Logf("Error while listing NodeLatencyStats: %v", 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.
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.
@antoninbas I've removed the redundant codes & implements Antrea framework more efficiently.
test/e2e/nodelatencymonitor_test.go
Outdated
valid, validateErr := validateNodeLatencyStats(statsList, true) | ||
if !valid { | ||
return false, validateErr | ||
} | ||
return true, nil |
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.
replace these 5 lines with return validateNodeLatencyStats(statsList, true)
test/e2e/nodelatencymonitor_test.go
Outdated
valid, validateErr := validateNodeLatencyStats(statsList, false) | ||
if !valid { | ||
return false, validateErr | ||
} | ||
return true, nil |
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.
replace these 5 lines with return validateNodeLatencyStats(statsList, false)
test/e2e/nodelatencymonitor_test.go
Outdated
err = data.crdClient.CrdV1alpha1().NodeLatencyMonitors().Delete(context.TODO(), "default", metav1.DeleteOptions{}) | ||
require.NoError(t, err, "Failed to delete NodeLatencyMonitor CR") | ||
t.Logf("NodeLatencyMonitor CR deleted successfully.") |
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 realized that this should be a defer
statement just after the Create
operation succeeds. Otherwise the test may fail (e.g., during one of the poll) and then the CR will not be deleted and will persist after the test case.
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 , you are right .
I'm changing.
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.
@antoninbas I have added he defer function that will surely delete nodeltancymonitor cr(if created) after test fail or success
@Rupam-It The DCO check is failing because at least one of your commits is not signed. This may be a good opportunity to squash all your commits and sign the one resulting commit. |
Signed-off-by: Rupam-It <[email protected]>
… tests for NodeLatencyMonitor resources to ensure better clarity and functionality. Signed-off-by: Rupam-It <[email protected]>
Signed-off-by: Rupam-It <[email protected]>
Signed-off-by: Rupam-It <[email protected]>
Signed-off-by: Rupam-It <[email protected]>
Signed-off-by: Rupam-It <[email protected]>
Signed-off-by: Rupam-It <[email protected]>
Signed-off-by: Rupam-It <[email protected]>
…me work Signed-off-by: Rupam-It <[email protected]>
Co-authored-by: Antonin Bas <[email protected]> Signed-off-by: Rupam-It <[email protected]>
Co-authored-by: Antonin Bas <[email protected]> Signed-off-by: Rupam-It <[email protected]>
Signed-off-by: Rupam-It <[email protected]>
8554278
to
3b0d16f
Compare
@antoninbas Thank you so much for your patience and guidance throughout this process. Can you suggest me next issue to work on ? |
@Rupam-It a few CI checks are failing:
|
Okk, I'm doing |
Signed-off-by: Rupam-It <[email protected]>
hey @antoninbas i have add the copywrite headers also change in code that passes CI checks . |
test/e2e/nodelatencymonitor_test.go
Outdated
package e2e | ||
|
||
import ( | ||
// Standard library 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.
this is not a helpful comment, please remove it
test/e2e/nodelatencymonitor_test.go
Outdated
@@ -1,16 +1,32 @@ | |||
// Copyright 2020 Antrea Authors |
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.
the date is wrong
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
Signed-off-by: Rupam-It <[email protected]>
I think this changes will works fine . |
/test-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.
LGTM - thank you for your contribution @Rupam-It !
/skip-conformance |
@antoninbas Thank you for your continuous guidance and patience throughout the process. I truly appreciate your support ! |
This PR adds end-to-end (e2e) tests for the NodeLatencyMonitor feature, which is currently available in Alpha as part of Antrea v2.1 (#6120).
Fixes #6549.