Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

adding a flag for csv timeout for check operator cmd #1198

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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"+
skattoju marked this conversation as resolved.
Show resolved Hide resolved
"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))
})
})
5 changes: 5 additions & 0 deletions internal/runtime/defaults.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
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,
acornett21 marked this conversation as resolved.
Show resolved Hide resolved
})
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
}
Loading