From 89aa5a882ba1425922f389aacb1421a8433b6b56 Mon Sep 17 00:00:00 2001 From: "Adam D. Cornett" Date: Mon, 16 Sep 2024 11:24:28 -0700 Subject: [PATCH] adding a flag for csv timeout for check operator cmd Signed-off-by: Adam D. Cornett --- cmd/preflight/cmd/check_operator.go | 10 +++++++++ cmd/preflight/cmd/root.go | 4 ++++ internal/engine/engine.go | 3 ++- internal/policy/operator/default.go | 2 -- internal/policy/operator/deployable_by_olm.go | 21 +++++++++++++++++-- .../policy/operator/deployable_by_olm_test.go | 3 +-- internal/runtime/config.go | 3 +++ internal/runtime/config_read.go | 6 ++++++ internal/runtime/config_read_test.go | 4 ++++ internal/runtime/config_test.go | 6 ++++-- internal/runtime/defaults.go | 5 +++++ operator/check_operator.go | 11 ++++++++++ 12 files changed, 69 insertions(+), 9 deletions(-) create mode 100644 internal/runtime/defaults.go diff --git a/cmd/preflight/cmd/check_operator.go b/cmd/preflight/cmd/check_operator.go index 098a855a..2919bd7a 100644 --- a/cmd/preflight/cmd/check_operator.go +++ b/cmd/preflight/cmd/check_operator.go @@ -52,6 +52,12 @@ func checkOperatorCmd(runpreflight runPreflight) *cobra.Command { "If empty, the default operator channel in bundle's annotations file is used.. (env: PFLT_CHANNEL)") _ = viper.BindPFlag("channel", checkOperatorCmd.Flags().Lookup("channel")) + checkOperatorCmd.Flags().Duration("csv-timeout", 0, "The Duration of time to wait for the ClusterServiceVersion to become healthy.\n"+ + "If empty the default of 180s will be used. (env: PFLT_CSV_TIMEOUT)") + _ = viper.BindPFlag("csv_timeout", checkOperatorCmd.Flags().Lookup("csv-timeout")) + + _ = checkOperatorCmd.Flags().MarkHidden("csv-timeout") + return checkOperatorCmd } @@ -169,6 +175,10 @@ func generateOperatorCheckOptions(cfg *runtime.Config) []operator.Option { opts = append(opts, operator.WithInsecureConnection()) } + if cfg.CSVTimeout != 0 { + opts = append(opts, operator.WithCSVTimeout(cfg.CSVTimeout)) + } + return opts } diff --git a/cmd/preflight/cmd/root.go b/cmd/preflight/cmd/root.go index 6559bcab..ec3419c7 100644 --- a/cmd/preflight/cmd/root.go +++ b/cmd/preflight/cmd/root.go @@ -8,6 +8,7 @@ import ( "path/filepath" "github.com/redhat-openshift-ecosystem/openshift-preflight/artifacts" + "github.com/redhat-openshift-ecosystem/openshift-preflight/internal/runtime" "github.com/redhat-openshift-ecosystem/openshift-preflight/internal/viper" "github.com/redhat-openshift-ecosystem/openshift-preflight/version" @@ -88,6 +89,9 @@ func initConfig(viper *spfviper.Viper) { // Set up scorecard wait time default viper.SetDefault("scorecard_wait_time", DefaultScorecardWaitTime) + + // Set up csv timout default + viper.SetDefault("csv_timeout", runtime.DefaultCSVTimeout) } // preRunConfig is used by cobra.PreRun in all non-root commands to load all necessary configurations diff --git a/internal/engine/engine.go b/internal/engine/engine.go index 244aafc5..ad278d16 100644 --- a/internal/engine/engine.go +++ b/internal/engine/engine.go @@ -722,6 +722,7 @@ type OperatorCheckConfig struct { ScorecardImage, ScorecardWaitTime, ScorecardNamespace, ScorecardServiceAccount string IndexImage, DockerConfig, Channel string Kubeconfig []byte + CSVTimeout time.Duration } // InitializeOperatorChecks returns opeartor checks for policy p give cfg. @@ -731,7 +732,7 @@ func InitializeOperatorChecks(ctx context.Context, p policy.Policy, cfg Operator return []check.Check{ operatorpol.NewScorecardBasicSpecCheck(operatorsdk.New(cfg.ScorecardImage, exec.Command), cfg.ScorecardNamespace, cfg.ScorecardServiceAccount, cfg.Kubeconfig, cfg.ScorecardWaitTime), operatorpol.NewScorecardOlmSuiteCheck(operatorsdk.New(cfg.ScorecardImage, exec.Command), cfg.ScorecardNamespace, cfg.ScorecardServiceAccount, cfg.Kubeconfig, cfg.ScorecardWaitTime), - operatorpol.NewDeployableByOlmCheck(cfg.IndexImage, cfg.DockerConfig, cfg.Channel), + operatorpol.NewDeployableByOlmCheck(cfg.IndexImage, cfg.DockerConfig, cfg.Channel, operatorpol.WithCSVTimeout(cfg.CSVTimeout)), operatorpol.NewValidateOperatorBundleCheck(), operatorpol.NewCertifiedImagesCheck(pyxis.NewPyxisClient( check.DefaultPyxisHost, diff --git a/internal/policy/operator/default.go b/internal/policy/operator/default.go index b60f8118..096caaa4 100644 --- a/internal/policy/operator/default.go +++ b/internal/policy/operator/default.go @@ -32,8 +32,6 @@ const ( var ( subscriptionTimeout time.Duration = 180 * time.Second - csvTimeout time.Duration = 180 * time.Second - approvedRegistries = map[string]struct{}{ "registry.connect.dev.redhat.com": {}, "registry.connect.qa.redhat.com": {}, diff --git a/internal/policy/operator/deployable_by_olm.go b/internal/policy/operator/deployable_by_olm.go index 3eba4649..673ee769 100644 --- a/internal/policy/operator/deployable_by_olm.go +++ b/internal/policy/operator/deployable_by_olm.go @@ -33,6 +33,8 @@ import ( crclient "sigs.k8s.io/controller-runtime/pkg/client" ) +type Option func(*DeployableByOlmCheck) + var _ check.Check = &DeployableByOlmCheck{} type operatorData struct { @@ -59,6 +61,7 @@ type DeployableByOlmCheck struct { client crclient.Client csvReady bool validImages bool + csvTimeout time.Duration } func (p *DeployableByOlmCheck) initClient() error { @@ -95,6 +98,14 @@ func (p *DeployableByOlmCheck) initOpenShifeEngine() { } } +// WithCSVTimeout overrides the default csvTimeout value, for operators that take +// additional time to install. +func WithCSVTimeout(csvTimeout time.Duration) Option { + return func(oc *DeployableByOlmCheck) { + oc.csvTimeout = csvTimeout + } +} + // NewDeployableByOlmCheck will return a check that validates if an operator // is deployable by OLM. An empty dockerConfig value implies that the images // in scope are public. An empty channel value implies that the check should @@ -103,12 +114,18 @@ func NewDeployableByOlmCheck( indexImage, dockerConfig, channel string, + opts ...Option, ) *DeployableByOlmCheck { - return &DeployableByOlmCheck{ + c := &DeployableByOlmCheck{ dockerConfig: dockerConfig, indexImage: indexImage, channel: channel, } + + for _, opt := range opts { + opt(c) + } + return c } func (p *DeployableByOlmCheck) Validate(ctx context.Context, bundleRef image.ImageReference) (bool, error) { @@ -491,7 +508,7 @@ func (p *DeployableByOlmCheck) isCSVReady(ctx context.Context, operatorData oper for _, CsvNamespace := range CsvNamespaces { wg.Add(1) - go watch(ctx, p.openshiftClient, &wg, operatorData.InstalledCsv, CsvNamespace, csvTimeout, csvChannel, csvStatusSucceeded) + go watch(ctx, p.openshiftClient, &wg, operatorData.InstalledCsv, CsvNamespace, p.csvTimeout, csvChannel, csvStatusSucceeded) } go func() { diff --git a/internal/policy/operator/deployable_by_olm_test.go b/internal/policy/operator/deployable_by_olm_test.go index b5fcd950..f9376800 100644 --- a/internal/policy/operator/deployable_by_olm_test.go +++ b/internal/policy/operator/deployable_by_olm_test.go @@ -29,7 +29,6 @@ var _ = Describe("DeployableByOLMCheck", func() { BeforeEach(func() { // override default timeout subscriptionTimeout = 1 * time.Second - csvTimeout = 1 * time.Second fakeImage := fakecranev1.FakeImage{} imageRef.ImageInfo = &fakeImage @@ -37,7 +36,7 @@ var _ = Describe("DeployableByOLMCheck", func() { now := metav1.Now() og.Status.LastUpdated = &now - deployableByOLMCheck = *NewDeployableByOlmCheck("test_indeximage", "", "") + deployableByOLMCheck = *NewDeployableByOlmCheck("test_indeximage", "", "", WithCSVTimeout(1*time.Second)) scheme := apiruntime.NewScheme() Expect(openshift.AddSchemes(scheme)).To(Succeed()) clientBuilder = fake.NewClientBuilder(). diff --git a/internal/runtime/config.go b/internal/runtime/config.go index 9fa991cd..fdcd8ea3 100644 --- a/internal/runtime/config.go +++ b/internal/runtime/config.go @@ -2,6 +2,7 @@ package runtime import ( "os" + "time" "github.com/redhat-openshift-ecosystem/openshift-preflight/internal/option" "github.com/redhat-openshift-ecosystem/openshift-preflight/internal/policy" @@ -37,6 +38,7 @@ type Config struct { Channel string IndexImage string Kubeconfig string + CSVTimeout time.Duration } // ReadOnly returns an uneditably configuration. @@ -83,6 +85,7 @@ func (c *Config) storeOperatorPolicyConfiguration(vcfg viper.Viper) { c.ScorecardWaitTime = vcfg.GetString("scorecard_wait_time") c.Channel = vcfg.GetString("channel") c.IndexImage = vcfg.GetString("indeximage") + c.CSVTimeout = vcfg.GetDuration("csv_timeout") } // This is to satisfy the CraneConfig interface diff --git a/internal/runtime/config_read.go b/internal/runtime/config_read.go index 1d8be133..456f5e92 100644 --- a/internal/runtime/config_read.go +++ b/internal/runtime/config_read.go @@ -1,6 +1,8 @@ package runtime import ( + "time" + "github.com/redhat-openshift-ecosystem/openshift-preflight/internal/config" "github.com/redhat-openshift-ecosystem/openshift-preflight/internal/policy" ) @@ -101,3 +103,7 @@ func (ro *ReadOnlyConfig) Platform() string { func (ro *ReadOnlyConfig) Insecure() bool { return ro.cfg.Insecure } + +func (ro *ReadOnlyConfig) CSVTimeout() time.Duration { + return ro.cfg.CSVTimeout +} diff --git a/internal/runtime/config_read_test.go b/internal/runtime/config_read_test.go index 128fb4eb..2caea5b1 100644 --- a/internal/runtime/config_read_test.go +++ b/internal/runtime/config_read_test.go @@ -1,6 +1,8 @@ package runtime import ( + "time" + . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" ) @@ -30,6 +32,7 @@ var _ = Describe("Runtime ReadOnlyConfig test", func() { Channel: "channel", IndexImage: "indeximg", Kubeconfig: "kubeconfig", + CSVTimeout: 180 * time.Second, } cro := c.ReadOnly() It("should return values assigned to corresponding struct fields", func() { @@ -55,6 +58,7 @@ var _ = Describe("Runtime ReadOnlyConfig test", func() { Expect(cro.Channel()).To(Equal("channel")) Expect(cro.IndexImage()).To(Equal("indeximg")) Expect(cro.Kubeconfig()).To(Equal("kubeconfig")) + Expect(cro.CSVTimeout()).To(Equal(180 * time.Second)) }) }) }) diff --git a/internal/runtime/config_test.go b/internal/runtime/config_test.go index 4593cc33..225ca07b 100644 --- a/internal/runtime/config_test.go +++ b/internal/runtime/config_test.go @@ -54,6 +54,8 @@ var _ = Describe("Viper to Runtime Config", func() { expectedRuntimeCfg.Channel = "mychannel" baseViperCfg.Set("indeximage", "myindeximage") expectedRuntimeCfg.IndexImage = "myindeximage" + baseViperCfg.Set("csv_timeout", DefaultCSVTimeout) + expectedRuntimeCfg.CSVTimeout = DefaultCSVTimeout }) Context("With values in a viper config", func() { @@ -64,12 +66,12 @@ var _ = Describe("Viper to Runtime Config", func() { }) }) - It("should only have 24 struct keys for tests to be valid", func() { + It("should only have 25 struct keys for tests to be valid", func() { // If this test fails, it means a developer has added or removed // keys from runtime.Config, and so these tests may no longer be // accurate in confirming that the derived configuration from viper // matches. keys := reflect.TypeOf(Config{}).NumField() - Expect(keys).To(Equal(24)) + Expect(keys).To(Equal(25)) }) }) diff --git a/internal/runtime/defaults.go b/internal/runtime/defaults.go new file mode 100644 index 00000000..e24fede3 --- /dev/null +++ b/internal/runtime/defaults.go @@ -0,0 +1,5 @@ +package runtime + +import "time" + +var DefaultCSVTimeout = 180 * time.Second diff --git a/operator/check_operator.go b/operator/check_operator.go index fdf6cf7d..ca6665dd 100644 --- a/operator/check_operator.go +++ b/operator/check_operator.go @@ -4,6 +4,7 @@ import ( "context" "fmt" goruntime "runtime" + "time" "github.com/redhat-openshift-ecosystem/openshift-preflight/certification" preflighterr "github.com/redhat-openshift-ecosystem/openshift-preflight/errors" @@ -92,6 +93,7 @@ func (c *operatorCheck) resolve(ctx context.Context) error { DockerConfig: c.dockerConfigFilePath, Channel: c.operatorChannel, Kubeconfig: c.kubeconfig, + CSVTimeout: c.csvTimeout, }) if err != nil { return fmt.Errorf("%w: %s", preflighterr.ErrCannotInitializeChecks, err) @@ -164,6 +166,14 @@ func WithInsecureConnection() Option { } } +// WithCSVTimeout overrides the default csvTimeout value, for operators that take +// additional time to install. +func WithCSVTimeout(csvTimeout time.Duration) Option { + return func(oc *operatorCheck) { + oc.csvTimeout = csvTimeout + } +} + type operatorCheck struct { // required image string @@ -180,4 +190,5 @@ type operatorCheck struct { checks []check.Check resolved bool policy policy.Policy + csvTimeout time.Duration }