Skip to content

Commit

Permalink
adding a flag for csv timeout for check operator cmd
Browse files Browse the repository at this point in the history
Signed-off-by: Adam D. Cornett <[email protected]>
  • Loading branch information
acornett21 committed Sep 30, 2024
1 parent 755831a commit c641cbf
Show file tree
Hide file tree
Showing 12 changed files with 71 additions and 9 deletions.
10 changes: 10 additions & 0 deletions cmd/preflight/cmd/check_operator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -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
}

Expand Down
4 changes: 4 additions & 0 deletions cmd/preflight/cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion internal/engine/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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,
Expand Down
2 changes: 0 additions & 2 deletions internal/policy/operator/default.go
Original file line number Diff line number Diff line change
Expand Up @@ -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": {},
Expand Down
21 changes: 19 additions & 2 deletions internal/policy/operator/deployable_by_olm.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ import (
crclient "sigs.k8s.io/controller-runtime/pkg/client"
)

type Option func(*DeployableByOlmCheck)

var _ check.Check = &DeployableByOlmCheck{}

type operatorData struct {
Expand All @@ -59,6 +61,7 @@ type DeployableByOlmCheck struct {
client crclient.Client
csvReady bool
validImages bool
csvTimeout time.Duration
}

func (p *DeployableByOlmCheck) initClient() error {
Expand Down Expand Up @@ -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
Expand All @@ -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) {
Expand Down Expand Up @@ -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() {
Expand Down
3 changes: 1 addition & 2 deletions internal/policy/operator/deployable_by_olm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,14 @@ var _ = Describe("DeployableByOLMCheck", func() {
BeforeEach(func() {
// override default timeout
subscriptionTimeout = 1 * time.Second
csvTimeout = 1 * time.Second

fakeImage := fakecranev1.FakeImage{}
imageRef.ImageInfo = &fakeImage
imageRef.ImageFSPath = "./testdata/all_namespaces"

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().
Expand Down
3 changes: 3 additions & 0 deletions internal/runtime/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -37,6 +38,7 @@ type Config struct {
Channel string
IndexImage string
Kubeconfig string
CSVTimeout time.Duration
}

// ReadOnly returns an uneditably configuration.
Expand Down Expand Up @@ -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
Expand Down
6 changes: 6 additions & 0 deletions internal/runtime/config_read.go
Original file line number Diff line number Diff line change
@@ -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"
)
Expand Down Expand Up @@ -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
}
4 changes: 4 additions & 0 deletions internal/runtime/config_read_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package runtime

import (
"time"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
)
Expand Down Expand Up @@ -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() {
Expand All @@ -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))
})
})
})
6 changes: 4 additions & 2 deletions internal/runtime/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -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))
})
})
7 changes: 7 additions & 0 deletions internal/runtime/defaults.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package runtime

import "time"

var (
DefaultCSVTimeout = 180 * time.Second
)
11 changes: 11 additions & 0 deletions operator/check_operator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand All @@ -180,4 +190,5 @@ type operatorCheck struct {
checks []check.Check
resolved bool
policy policy.Policy
csvTimeout time.Duration
}

0 comments on commit c641cbf

Please sign in to comment.