From 48f5c7aba638c0f8e92a575ce0168a71021f806e Mon Sep 17 00:00:00 2001 From: Kumar Atish Date: Thu, 9 Mar 2023 14:14:11 +0530 Subject: [PATCH] Fix e2e TestAntctl on windows testbed (#4663) TestAntctl/testAntctlControllerRemoteAccess failed on all windows testbed because it cannot get output of `antctl get memberlist` while collecting supportbundle from outside on windows testbed as memberlist is not running. This PR handles the above case as success on linux and writes output to file that memberlist is not running. On Windows, don't collect memberlist as it never runs on Windows. Fixes #4659 Signed-off-by: Kumar Atish --- .../apiserver/handlers/memberlist/handler.go | 8 +- .../handlers/memberlist/handler_test.go | 88 +++++++++++++------ pkg/support/dump.go | 4 - pkg/support/dump_others.go | 9 ++ pkg/support/dump_windows.go | 5 ++ 5 files changed, 83 insertions(+), 31 deletions(-) diff --git a/pkg/agent/apiserver/handlers/memberlist/handler.go b/pkg/agent/apiserver/handlers/memberlist/handler.go index 3548a3ab59c..f075922cd08 100644 --- a/pkg/agent/apiserver/handlers/memberlist/handler.go +++ b/pkg/agent/apiserver/handlers/memberlist/handler.go @@ -17,6 +17,7 @@ package memberlist import ( "encoding/json" "net/http" + "reflect" v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/labels" @@ -48,9 +49,14 @@ func generateResponse(node *v1.Node, aliveNodes sets.String) Response { // HandleFunc returns the function which can handle queries issued by the memberlist command. func HandleFunc(aq querier.AgentQuerier) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { + memberlistCluster := aq.GetMemberlistCluster() + if reflect.ValueOf(memberlistCluster).IsNil() { + http.Error(w, "memberlist is not running", http.StatusServiceUnavailable) + return + } var memberlist []Response allNodes, _ := aq.GetNodeLister().List(labels.Everything()) - aliveNodes := aq.GetMemberlistCluster().AliveNodes() + aliveNodes := memberlistCluster.AliveNodes() for _, node := range allNodes { memberlist = append(memberlist, generateResponse(node, aliveNodes)) } diff --git a/pkg/agent/apiserver/handlers/memberlist/handler_test.go b/pkg/agent/apiserver/handlers/memberlist/handler_test.go index d6c965df110..0b1ddd7ce6d 100644 --- a/pkg/agent/apiserver/handlers/memberlist/handler_test.go +++ b/pkg/agent/apiserver/handlers/memberlist/handler_test.go @@ -28,7 +28,9 @@ import ( "k8s.io/apimachinery/pkg/util/sets" "k8s.io/client-go/informers" "k8s.io/client-go/kubernetes/fake" + corelisters "k8s.io/client-go/listers/core/v1" + "antrea.io/antrea/pkg/agent/memberlist" memberlisttest "antrea.io/antrea/pkg/agent/memberlist/testing" queriertest "antrea.io/antrea/pkg/agent/querier/testing" ) @@ -68,35 +70,69 @@ func TestMemberlistQuery(t *testing.T) { informerFactory.Start(stopCh) informerFactory.WaitForCacheSync(stopCh) - ctrl := gomock.NewController(t) - q := queriertest.NewMockAgentQuerier(ctrl) - memberlistInterface := memberlisttest.NewMockInterface(ctrl) - q.EXPECT().GetNodeLister().Return(nodeLister) - q.EXPECT().GetMemberlistCluster().Return(memberlistInterface) - memberlistInterface.EXPECT().AliveNodes().Return(sets.NewString("node1")) - handler := HandleFunc(q) - - req, err := http.NewRequest(http.MethodGet, "", nil) - require.NoError(t, err) - - recorder := httptest.NewRecorder() - handler.ServeHTTP(recorder, req) - assert.Equal(t, http.StatusOK, recorder.Code) - - expectedResponse := []Response{ + tests := []struct { + name string + memberlistInterface func(*gomock.Controller) memberlist.Interface + nodeLister corelisters.NodeLister + expectedStatus int + expectedResponse []Response + }{ { - NodeName: "node1", - IP: "172.16.0.11", - Status: "Alive", + name: "memberlist not running", + memberlistInterface: func(c *gomock.Controller) memberlist.Interface { + var cluster *memberlist.Cluster + var m memberlist.Interface = cluster + return m + }, + expectedStatus: http.StatusServiceUnavailable, }, { - NodeName: "node2", - IP: "172.16.0.12", - Status: "Dead", + name: "get memberlist", + memberlistInterface: func(c *gomock.Controller) memberlist.Interface { + m := memberlisttest.NewMockInterface(c) + m.EXPECT().AliveNodes().Return(sets.NewString("node1")) + return m + }, + nodeLister: nodeLister, + expectedStatus: http.StatusOK, + expectedResponse: []Response{ + { + NodeName: "node1", + IP: "172.16.0.11", + Status: "Alive", + }, + { + NodeName: "node2", + IP: "172.16.0.12", + Status: "Dead", + }, + }, }, } - var received []Response - err = json.Unmarshal(recorder.Body.Bytes(), &received) - require.NoError(t, err) - assert.ElementsMatch(t, expectedResponse, received) + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctrl := gomock.NewController(t) + q := queriertest.NewMockAgentQuerier(ctrl) + q.EXPECT().GetMemberlistCluster().Return(tt.memberlistInterface(ctrl)) + if tt.nodeLister != nil { + q.EXPECT().GetNodeLister().Return(tt.nodeLister) + } + handler := HandleFunc(q) + + req, err := http.NewRequest(http.MethodGet, "", nil) + require.NoError(t, err) + + recorder := httptest.NewRecorder() + handler.ServeHTTP(recorder, req) + assert.Equal(t, tt.expectedStatus, recorder.Code) + + if tt.expectedStatus == http.StatusOK { + var received []Response + err = json.Unmarshal(recorder.Body.Bytes(), &received) + require.NoError(t, err) + assert.ElementsMatch(t, tt.expectedResponse, received) + } + }) + } } diff --git a/pkg/support/dump.go b/pkg/support/dump.go index 598c3707a84..650b4342bc9 100644 --- a/pkg/support/dump.go +++ b/pkg/support/dump.go @@ -292,10 +292,6 @@ func (d *agentDumper) DumpAgentInfo(basedir string) error { return writeYAMLFile(d.fs, filepath.Join(basedir, "agentinfo"), "agentinfo", ai) } -func (d *agentDumper) DumpMemberlist(basedir string) error { - return dumpAntctlGet(d.fs, d.executor, "memberlist", basedir) -} - func (d *agentDumper) DumpNetworkPolicyResources(basedir string) error { dump := func(o interface{}, name string) error { return writeYAMLFile(d.fs, filepath.Join(basedir, name), name, o) diff --git a/pkg/support/dump_others.go b/pkg/support/dump_others.go index b3b9e061c7a..b0ddf37f589 100644 --- a/pkg/support/dump_others.go +++ b/pkg/support/dump_others.go @@ -21,6 +21,7 @@ import ( "fmt" "path" "path/filepath" + "strings" "antrea.io/antrea/pkg/agent/util/iptables" "antrea.io/antrea/pkg/util/logdir" @@ -73,3 +74,11 @@ func (d *agentDumper) dumpIPToolInfo(basedir string) error { } return nil } + +func (d *agentDumper) DumpMemberlist(basedir string) error { + output, err := d.executor.Command("antctl", "-oyaml", "get", "memberlist").CombinedOutput() + if err != nil && !strings.Contains(string(output), "memberlist is not running") { + return fmt.Errorf("error when dumping memberlist: %w", err) + } + return writeFile(d.fs, filepath.Join(basedir, "memberlist"), "memberlist", output) +} diff --git a/pkg/support/dump_windows.go b/pkg/support/dump_windows.go index 560851a5f83..5bb2d6ff02f 100644 --- a/pkg/support/dump_windows.go +++ b/pkg/support/dump_windows.go @@ -101,3 +101,8 @@ func (d *agentDumper) dumpHNSResources(basedir string) error { } return nil } + +func (d *agentDumper) DumpMemberlist(basedir string) error { + // memberlist never runs on windows. + return nil +}