diff --git a/internal/onetime/status/status.go b/internal/onetime/status/status.go index b31ef15d..732d4430 100644 --- a/internal/onetime/status/status.go +++ b/internal/onetime/status/status.go @@ -116,12 +116,13 @@ type ( // Status stores the status subcommand parameters. type Status struct { - ConfigFilePath string - BackintParametersPath string - Feature string - CloudProps *iipb.CloudProperties - HeartbeatSpec *heartbeat.Spec - WLMService WLMInterface + ConfigFilePath string + BackintParametersPath string + Feature string + CloudProps *iipb.CloudProperties + HeartbeatSpec *heartbeat.Spec + WLMService WLMInterface + NoDiskSnapshotStatusCheck bool compact bool help bool @@ -295,8 +296,28 @@ func start(ctx context.Context, a any) { } } +// Creates a new string from given comma separated string without the specified element. +func removeStringElement(str string, elementToRemove string) string { + slice := strings.Split(str, ",") + var result []string + for _, item := range slice { + if item != elementToRemove { + result = append(result, item) + } + } + return strings.Join(result, ",") +} + func (s *Status) collectAndSendStatus(ctx context.Context) error { s.HeartbeatSpec.Beat() + + // If the disk snapshot status check is disabled, remove the disk snapshot feature from the list + // of features to check. + if s.NoDiskSnapshotStatusCheck { + s.Feature = removeStringElement(allFeatures, diskSnapshot) + log.CtxLogger(ctx).Debug("Disk snapshot status check DISABLED for daemon") + } + agentStatus, err := s.statusHandler(ctx) if err != nil { log.CtxLogger(ctx).Errorw("Could not get agent status", "error", err) diff --git a/internal/onetime/status/status_test.go b/internal/onetime/status/status_test.go index 2db45f19..74f816f1 100644 --- a/internal/onetime/status/status_test.go +++ b/internal/onetime/status/status_test.go @@ -42,6 +42,7 @@ import ( "github.com/google/subcommands" "github.com/GoogleCloudPlatform/sapagent/internal/configuration" "github.com/GoogleCloudPlatform/sapagent/internal/databaseconnector" + "github.com/GoogleCloudPlatform/sapagent/internal/heartbeat" "github.com/GoogleCloudPlatform/sapagent/internal/iam" "github.com/GoogleCloudPlatform/sapagent/shared/iam" "github.com/GoogleCloudPlatform/workloadagentplatform/sharedlibraries/commandlineexecutor" @@ -310,9 +311,10 @@ func TestStartStatusCollection(t *testing.T) { func TestCollectAndSendStatus(t *testing.T) { tests := []struct { - name string - s Status - want error + name string + s Status + want error + wantDiskSnapshotInFeatureFlag bool }{ { name: "SuccessEmptyConfig", @@ -339,6 +341,7 @@ func TestCollectAndSendStatus(t *testing.T) { Zone: "us-central1-a", Region: "test-region", }, + HeartbeatSpec: &heartbeat.Spec{BeatFunc: func() {}}, httpGet: httpGetSuccess, createDBHandle: dbConnectorSuccess, iamService: &iam.IAM{}, @@ -353,7 +356,8 @@ func TestCollectAndSendStatus(t *testing.T) { WriteInsightErrs: []error{nil}, }, }, - want: nil, + want: nil, + wantDiskSnapshotInFeatureFlag: true, // Default behavior }, { name: "SuccessAllEnabled", @@ -405,6 +409,7 @@ func TestCollectAndSendStatus(t *testing.T) { Zone: "us-central1-a", Region: "test-region", }, + HeartbeatSpec: &heartbeat.Spec{BeatFunc: func() {}}, stat: func(name string) (os.FileInfo, error) { return &mockFileInfo{perm: 0400}, nil }, @@ -429,11 +434,12 @@ func TestCollectAndSendStatus(t *testing.T) { WriteInsightErrs: []error{nil}, }, }, - want: nil, + want: nil, + wantDiskSnapshotInFeatureFlag: true, // Default behavior }, { name: "StatusStructNotInitialized", - s: Status{}, + s: Status{HeartbeatSpec: &heartbeat.Spec{BeatFunc: func() {}}}, want: cmpopts.AnyError, }, { @@ -461,6 +467,7 @@ func TestCollectAndSendStatus(t *testing.T) { Zone: "us-central1-a", Region: "test-region", }, + HeartbeatSpec: &heartbeat.Spec{BeatFunc: func() {}}, httpGet: httpGetSuccess, createDBHandle: dbConnectorSuccess, iamService: &iam.IAM{}, @@ -475,15 +482,50 @@ func TestCollectAndSendStatus(t *testing.T) { WriteInsightErrs: []error{cmpopts.AnyError}, }, }, - want: cmpopts.AnyError, + want: cmpopts.AnyError, + wantDiskSnapshotInFeatureFlag: true, + }, + { + name: "DiskSnapshotCheckDisabledByFlag", + s: Status{ + NoDiskSnapshotStatusCheck: true, // Disable disk snapshot status check + readFile: func(string) ([]byte, error) { return nil, nil }, + exec: func(context.Context, commandlineexecutor.Params) commandlineexecutor.Result { + return commandlineexecutor.Result{StdOut: "enabled", ExitCode: 0} + }, + CloudProps: &iipb.CloudProperties{Scopes: []string{requiredScope}, Zone: "us-central1-a", Region: "test-region"}, + HeartbeatSpec: &heartbeat.Spec{BeatFunc: func() {}}, + iamService: &iam.IAM{}, + arClient: fakeArtifactRegistryClient([]string{""}), + permissionsStatus: func(ctx context.Context, iamService permissions.IAMService, serviceName string, r *permissions.ResourceDetails) (map[string]bool, error) { + return map[string]bool{"monitoring.timeSeries.create": true}, nil + }, + WLMService: &fake.TestWLM{WriteInsightErrs: []error{nil}}, + // Add other necessary mocks if statusHandler call depends on them + httpGet: httpGetSuccess, + createDBHandle: dbConnectorSuccess, + stat: func(name string) (os.FileInfo, error) { return &mockFileInfo{perm: 0777}, nil }, + readDir: func(dirname string) ([]fs.FileInfo, error) { return []fs.FileInfo{&mockFileInfo{perm: 0777}}, nil }, + }, + want: nil, + wantDiskSnapshotInFeatureFlag: false, // Expect disk_snapshot to be removed }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { t.Parallel() + test.s.Feature = allFeatures got := test.s.collectAndSendStatus(context.Background()) if !cmp.Equal(got, test.want, cmpopts.EquateErrors()) { - t.Errorf("statusHandler()=%v want %v", got, test.want) + t.Errorf("collectAndSendStatus()=%v want %v", got, test.want) + } + + if test.want == nil { + var featuresSeenByHandler string = test.s.Feature + hasDiskSnapshot := strings.Contains(featuresSeenByHandler, diskSnapshot) + if hasDiskSnapshot != test.wantDiskSnapshotInFeatureFlag { + t.Errorf("collectAndSendStatus() features string: %q, contains disk_snapshot: %v, want: %v", featuresSeenByHandler, hasDiskSnapshot, test.wantDiskSnapshotInFeatureFlag) + } } }) } diff --git a/internal/startdaemon/startdaemon.go b/internal/startdaemon/startdaemon.go index bfeb1d55..9d6792e2 100644 --- a/internal/startdaemon/startdaemon.go +++ b/internal/startdaemon/startdaemon.go @@ -109,10 +109,11 @@ var ( // Daemon has args for startdaemon subcommand. type Daemon struct { - configFilePath string - lp log.Parameters - config *cpb.Configuration - cloudProps *iipb.CloudProperties + configFilePath string + lp log.Parameters + config *cpb.Configuration + cloudProps *iipb.CloudProperties + noDiskSnapshotStatusCheck bool } // Name implements the subcommand interface for startdaemon. @@ -123,13 +124,14 @@ func (*Daemon) Synopsis() string { return "start daemon mode of the agent" } // Usage implements the subcommand interface for startdaemon. func (*Daemon) Usage() string { - return "Usage: startdaemon [-config ] \n" + return "Usage: startdaemon [-config ] [-no_disk_snapshot_status_check]\n" } // SetFlags implements the subcommand interface for startdaemon. func (d *Daemon) SetFlags(fs *flag.FlagSet) { fs.StringVar(&d.configFilePath, "config", "", "configuration path for startdaemon mode") fs.StringVar(&d.configFilePath, "c", "", "configuration path for startdaemon mode") + fs.BoolVar(&d.noDiskSnapshotStatusCheck, "no_disk_snapshot_status_check", false, "suppress disk_snapshot status checks in daemon mode") } // Execute implements the subcommand interface for startdaemon. @@ -483,9 +485,10 @@ func (d *Daemon) startServices(ctx context.Context, cancel context.CancelFunc, g // Start Status Collection statusCtx := log.SetCtx(ctx, "context", "Status") sp := StatusParams{&status.Status{ - ConfigFilePath: d.configFilePath, - CloudProps: d.cloudProps, - WLMService: wlmService}, healthMonitor} + ConfigFilePath: d.configFilePath, + CloudProps: d.cloudProps, + WLMService: wlmService, + NoDiskSnapshotStatusCheck: d.noDiskSnapshotStatusCheck}, healthMonitor} sp.startCollection(statusCtx) waitForShutdown(ctx, shutdownch, cancel, restarting)