diff --git a/pkg/apiserver/registry/system/supportbundle/rest.go b/pkg/apiserver/registry/system/supportbundle/rest.go index 55dbf4ed84f..3c928fb0e96 100644 --- a/pkg/apiserver/registry/system/supportbundle/rest.go +++ b/pkg/apiserver/registry/system/supportbundle/rest.go @@ -50,8 +50,10 @@ const ( ) var ( + // Declared as variables for testing. defaultFS = afero.NewOsFs() defaultExecutor = exec.New() + newAgentDumper = support.NewAgentDumper ) // NewControllerStorage creates a support bundle storage for working on antrea controller. @@ -164,7 +166,7 @@ func (r *supportBundleREST) Create(ctx context.Context, obj runtime.Object, _ re } }() - if err != nil { + if err == nil { r.clean(ctx, b.Filepath, bundleExpireDuration) } }(r.cache.Since) @@ -257,7 +259,7 @@ func (r *supportBundleREST) collect(ctx context.Context, dumpers ...func(string) } func (r *supportBundleREST) collectAgent(ctx context.Context, since string) (*systemv1beta1.SupportBundle, error) { - dumper := support.NewAgentDumper(defaultFS, defaultExecutor, r.ovsCtlClient, r.aq, r.npq, since, r.v4Enabled, r.v6Enabled) + dumper := newAgentDumper(defaultFS, defaultExecutor, r.ovsCtlClient, r.aq, r.npq, since, r.v4Enabled, r.v6Enabled) return r.collect( ctx, dumper.DumpLog, diff --git a/pkg/apiserver/registry/system/supportbundle/rest_test.go b/pkg/apiserver/registry/system/supportbundle/rest_test.go index 5a372ec21e2..704993dfa06 100644 --- a/pkg/apiserver/registry/system/supportbundle/rest_test.go +++ b/pkg/apiserver/registry/system/supportbundle/rest_test.go @@ -21,6 +21,7 @@ import ( "testing" "time" + "github.com/golang/mock/gomock" "github.com/spf13/afero" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -29,7 +30,14 @@ import ( "k8s.io/utils/exec" exectesting "k8s.io/utils/exec/testing" + agentquerier "antrea.io/antrea/pkg/agent/querier" + agentqueriertest "antrea.io/antrea/pkg/agent/querier/testing" system "antrea.io/antrea/pkg/apis/system/v1beta1" + "antrea.io/antrea/pkg/ovs/ovsctl" + ovsctltest "antrea.io/antrea/pkg/ovs/ovsctl/testing" + "antrea.io/antrea/pkg/querier" + queriertest "antrea.io/antrea/pkg/querier/testing" + "antrea.io/antrea/pkg/support" ) type testExec struct { @@ -149,11 +157,12 @@ func TestControllerStorage(t *testing.T) { collectedBundle = object.(*system.SupportBundle) return collectedBundle.Status == system.SupportBundleStatusCollected }, time.Second*2, time.Millisecond*100) - require.NotEmpty(t, collectedBundle.Filepath) - defer defaultFS.Remove(collectedBundle.Filepath) + filePath := collectedBundle.Filepath + require.NotEmpty(t, filePath) + defer defaultFS.Remove(filePath) assert.NotEmpty(t, collectedBundle.Sum) assert.Greater(t, collectedBundle.Size, uint32(0)) - exist, err := afero.Exists(defaultFS, collectedBundle.Filepath) + exist, err := afero.Exists(defaultFS, filePath) require.NoError(t, err) require.True(t, exist) @@ -173,4 +182,148 @@ func TestControllerStorage(t *testing.T) { ObjectMeta: metav1.ObjectMeta{Name: modeController}, Status: system.SupportBundleStatusNone, }, object) + assert.Eventuallyf(t, func() bool { + exist, err := afero.Exists(defaultFS, filePath) + require.NoError(t, err) + return !exist + }, time.Second*2, time.Millisecond*100, "Supportbundle file %s was not deleted after deleting the Supportbundle object", filePath) +} + +type fakeAgentDumper struct { + returnErr error +} + +func (f *fakeAgentDumper) DumpFlows(basedir string) error { + return f.returnErr +} + +func (f *fakeAgentDumper) DumpHostNetworkInfo(basedir string) error { + return f.returnErr +} + +func (f *fakeAgentDumper) DumpLog(basedir string) error { + return f.returnErr +} + +func (f *fakeAgentDumper) DumpAgentInfo(basedir string) error { + return f.returnErr +} + +func (f *fakeAgentDumper) DumpNetworkPolicyResources(basedir string) error { + return f.returnErr +} + +func (f *fakeAgentDumper) DumpHeapPprof(basedir string) error { + return f.returnErr +} + +func (f *fakeAgentDumper) DumpOVSPorts(basedir string) error { + return f.returnErr +} + +func TestAgentStorage(t *testing.T) { + defaultFS = afero.NewMemMapFs() + defaultExecutor = new(testExec) + newAgentDumper = func(fs afero.Fs, executor exec.Interface, ovsCtlClient ovsctl.OVSCtlClient, aq agentquerier.AgentQuerier, npq querier.AgentNetworkPolicyInfoQuerier, since string, v4Enabled, v6Enabled bool) support.AgentDumper { + return &fakeAgentDumper{} + } + defer func() { + defaultFS = afero.NewOsFs() + defaultExecutor = exec.New() + newAgentDumper = support.NewAgentDumper + }() + + ctx := context.Background() + ctrl := gomock.NewController(t) + defer ctrl.Finish() + fakeOVSCtl := ovsctltest.NewMockOVSCtlClient(ctrl) + fakeAgentQuerier := agentqueriertest.NewMockAgentQuerier(ctrl) + fakeNetworkPolicyQuerier := queriertest.NewMockAgentNetworkPolicyInfoQuerier(ctrl) + storage := NewAgentStorage(fakeOVSCtl, fakeAgentQuerier, fakeNetworkPolicyQuerier, true, true) + _, err := storage.SupportBundle.Create(ctx, &system.SupportBundle{ + ObjectMeta: metav1.ObjectMeta{ + Name: modeAgent, + }, + Status: system.SupportBundleStatusNone, + Since: "-1h", + }, nil, nil) + require.NoError(t, err) + + var collectedBundle *system.SupportBundle + assert.Eventually(t, func() bool { + object, err := storage.SupportBundle.Get(ctx, modeAgent, nil) + require.NoError(t, err) + collectedBundle = object.(*system.SupportBundle) + return collectedBundle.Status == system.SupportBundleStatusCollected + }, time.Second*2, time.Millisecond*100) + require.NotEmpty(t, collectedBundle.Filepath) + filePath := collectedBundle.Filepath + defer defaultFS.Remove(filePath) + assert.NotEmpty(t, collectedBundle.Sum) + assert.Greater(t, collectedBundle.Size, uint32(0)) + exist, err := afero.Exists(defaultFS, filePath) + require.NoError(t, err) + require.True(t, exist) + + _, deleted, err := storage.SupportBundle.Delete(ctx, modeAgent, nil, nil) + assert.NoError(t, err) + assert.True(t, deleted) + object, err := storage.SupportBundle.Get(ctx, modeAgent, nil) + assert.NoError(t, err) + assert.Equal(t, &system.SupportBundle{ + ObjectMeta: metav1.ObjectMeta{Name: modeAgent}, + Status: system.SupportBundleStatusNone, + }, object) + assert.Eventuallyf(t, func() bool { + exist, err := afero.Exists(defaultFS, filePath) + require.NoError(t, err) + return !exist + }, time.Second*2, time.Millisecond*100, "Supportbundle file %s was not deleted after deleting the Supportbundle object", filePath) +} + +func TestAgentStorageFailure(t *testing.T) { + defaultFS = afero.NewMemMapFs() + defaultExecutor = new(testExec) + newAgentDumper = func(fs afero.Fs, executor exec.Interface, ovsCtlClient ovsctl.OVSCtlClient, aq agentquerier.AgentQuerier, npq querier.AgentNetworkPolicyInfoQuerier, since string, v4Enabled, v6Enabled bool) support.AgentDumper { + return &fakeAgentDumper{returnErr: fmt.Errorf("iptables not found")} + } + defer func() { + defaultFS = afero.NewOsFs() + defaultExecutor = exec.New() + newAgentDumper = support.NewAgentDumper + }() + + ctx := context.Background() + ctrl := gomock.NewController(t) + defer ctrl.Finish() + fakeOVSCtl := ovsctltest.NewMockOVSCtlClient(ctrl) + fakeAgentQuerier := agentqueriertest.NewMockAgentQuerier(ctrl) + fakeNetworkPolicyQuerier := queriertest.NewMockAgentNetworkPolicyInfoQuerier(ctrl) + + storage := NewAgentStorage(fakeOVSCtl, fakeAgentQuerier, fakeNetworkPolicyQuerier, true, true) + _, err := storage.SupportBundle.Create(ctx, &system.SupportBundle{ + ObjectMeta: metav1.ObjectMeta{ + Name: modeAgent, + }, + Status: system.SupportBundleStatusNone, + Since: "-1h", + }, nil, nil) + require.NoError(t, err) + + var collectedBundle *system.SupportBundle + assert.Eventually(t, func() bool { + object, err := storage.SupportBundle.Get(ctx, modeAgent, nil) + require.NoError(t, err) + collectedBundle = object.(*system.SupportBundle) + return collectedBundle.Status == system.SupportBundleStatusNone + }, time.Second*2, time.Millisecond*100) + assert.Empty(t, collectedBundle.Filepath) + assert.Empty(t, collectedBundle.Sum) + assert.Empty(t, collectedBundle.Size) + + _, err = storage.SupportBundle.Get(ctx, modeController, nil) + assert.Equal(t, errors.NewNotFound(system.Resource("supportBundle"), modeController), err) + _, deleted, err := storage.SupportBundle.Delete(ctx, modeController, nil, nil) + assert.Equal(t, errors.NewNotFound(system.Resource("supportBundle"), modeController), err) + assert.False(t, deleted) }