Skip to content

Commit

Permalink
adding a flag for subscription timeout for check operator cmd
Browse files Browse the repository at this point in the history
Signed-off-by: Adam D. Cornett <adc@redhat.com>
  • Loading branch information
acornett21 committed Oct 15, 2024
1 parent 6dad285 commit 1cace8c
Show file tree
Hide file tree
Showing 10 changed files with 61 additions and 30 deletions.
9 changes: 9 additions & 0 deletions cmd/preflight/cmd/check_operator.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,12 @@ func checkOperatorCmd(runpreflight runPreflight) *cobra.Command {
"If empty the default of 180s will be used. (env: PFLT_CSV_TIMEOUT)")
_ = viper.BindPFlag("csv_timeout", checkOperatorCmd.Flags().Lookup("csv-timeout"))

checkOperatorCmd.Flags().Duration("subscription-timeout", 0, "The Duration of time to wait for the Subscription to become healthy.\n"+
"If empty the default of 180s will be used. (env: PFLT_SUBSCRIPTION_TIMEOUT)")
_ = viper.BindPFlag("subscription_timeout", checkOperatorCmd.Flags().Lookup("subscription-timeout"))

_ = checkOperatorCmd.Flags().MarkHidden("csv-timeout")
_ = checkOperatorCmd.Flags().MarkHidden("subscription-timeout")

return checkOperatorCmd
}
Expand Down Expand Up @@ -179,6 +184,10 @@ func generateOperatorCheckOptions(cfg *runtime.Config) []operator.Option {
opts = append(opts, operator.WithCSVTimeout(cfg.CSVTimeout))
}

if cfg.SubscriptionTimeout != 0 {
opts = append(opts, operator.WithSubscriptionTimeout(cfg.SubscriptionTimeout))
}

return opts
}

Expand Down
3 changes: 3 additions & 0 deletions cmd/preflight/cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,9 @@ func initConfig(viper *spfviper.Viper) {

// Set up csv timout default
viper.SetDefault("csv_timeout", runtime.DefaultCSVTimeout)

// Set up subscription timeout default
viper.SetDefault("subscription_timeout", runtime.DefaultSubscriptionTimeout)
}

// 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 @@ -723,6 +723,7 @@ type OperatorCheckConfig struct {
IndexImage, DockerConfig, Channel string
Kubeconfig []byte
CSVTimeout time.Duration
SubscriptionTimeout time.Duration
}

// InitializeOperatorChecks returns opeartor checks for policy p give cfg.
Expand All @@ -732,7 +733,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.WithCSVTimeout(cfg.CSVTimeout)),
operatorpol.NewDeployableByOlmCheck(cfg.IndexImage, cfg.DockerConfig, cfg.Channel, operatorpol.WithCSVTimeout(cfg.CSVTimeout), operatorpol.WithSubscriptionTimeout(cfg.SubscriptionTimeout)),
operatorpol.NewValidateOperatorBundleCheck(),
operatorpol.NewCertifiedImagesCheck(pyxis.NewPyxisClient(
check.DefaultPyxisHost,
Expand Down
4 changes: 0 additions & 4 deletions internal/policy/operator/default.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
package operator

import (
"time"

operatorsv1alpha1 "github.com/operator-framework/api/pkg/operators/v1alpha1"
)

Expand Down Expand Up @@ -30,8 +28,6 @@ const (
)

var (
subscriptionTimeout time.Duration = 180 * time.Second

approvedRegistries = map[string]struct{}{
"registry.connect.dev.redhat.com": {},
"registry.connect.qa.redhat.com": {},
Expand Down
20 changes: 14 additions & 6 deletions internal/policy/operator/deployable_by_olm.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,12 @@ type DeployableByOlmCheck struct {
// channel is optional. If empty, we will introspect.
channel string

openshiftClient openshift.Client
client crclient.Client
csvReady bool
validImages bool
csvTimeout time.Duration
openshiftClient openshift.Client
client crclient.Client
csvReady bool
validImages bool
csvTimeout time.Duration
subscriptionTimeout time.Duration
}

func (p *DeployableByOlmCheck) initClient() error {
Expand Down Expand Up @@ -106,6 +107,13 @@ func WithCSVTimeout(csvTimeout time.Duration) Option {
}
}

// WithSubscriptionTimeout customizes how long to wait for a subscription to become healthy.
func WithSubscriptionTimeout(subscriptionTimeout time.Duration) Option {
return func(oc *DeployableByOlmCheck) {
oc.subscriptionTimeout = subscriptionTimeout
}
}

// 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 Down Expand Up @@ -549,7 +557,7 @@ func (p *DeployableByOlmCheck) installedCSV(ctx context.Context, operatorData op
var wg sync.WaitGroup
// query API server for the installed CSV field of the created subscription
wg.Add(1)
go watch(ctx, p.openshiftClient, &wg, operatorData.App, operatorData.InstallNamespace, subscriptionTimeout, installedCSVChannel, subscriptionCsvIsInstalled)
go watch(ctx, p.openshiftClient, &wg, operatorData.App, operatorData.InstallNamespace, p.subscriptionTimeout, installedCSVChannel, subscriptionCsvIsInstalled)

go func() {
wg.Wait()
Expand Down
5 changes: 1 addition & 4 deletions internal/policy/operator/deployable_by_olm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,16 +27,13 @@ var _ = Describe("DeployableByOLMCheck", func() {
)

BeforeEach(func() {
// override default timeout
subscriptionTimeout = 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", "", "", WithCSVTimeout(1*time.Second))
deployableByOLMCheck = *NewDeployableByOlmCheck("test_indeximage", "", "", WithCSVTimeout(1*time.Second), WithSubscriptionTimeout(1*time.Second))
scheme := apiruntime.NewScheme()
Expect(openshift.AddSchemes(scheme)).To(Succeed())
clientBuilder = fake.NewClientBuilder().
Expand Down
18 changes: 10 additions & 8 deletions internal/runtime/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,15 @@ type Config struct {
Offline bool
ManifestListDigest string
// Operator-Specific Fields
Namespace string
ServiceAccount string
ScorecardImage string
ScorecardWaitTime string
Channel string
IndexImage string
Kubeconfig string
CSVTimeout time.Duration
Namespace string
ServiceAccount string
ScorecardImage string
ScorecardWaitTime string
Channel string
IndexImage string
Kubeconfig string
CSVTimeout time.Duration
SubscriptionTimeout time.Duration
}

// ReadOnly returns an uneditably configuration.
Expand Down Expand Up @@ -86,6 +87,7 @@ func (c *Config) storeOperatorPolicyConfiguration(vcfg viper.Viper) {
c.Channel = vcfg.GetString("channel")
c.IndexImage = vcfg.GetString("indeximage")
c.CSVTimeout = vcfg.GetDuration("csv_timeout")
c.SubscriptionTimeout = vcfg.GetDuration("subscription_timeout")
}

// This is to satisfy the CraneConfig interface
Expand Down
6 changes: 4 additions & 2 deletions internal/runtime/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@ var _ = Describe("Viper to Runtime Config", func() {
expectedRuntimeCfg.IndexImage = "myindeximage"
baseViperCfg.Set("csv_timeout", DefaultCSVTimeout)
expectedRuntimeCfg.CSVTimeout = DefaultCSVTimeout
baseViperCfg.Set("subscription_timeout", DefaultSubscriptionTimeout)
expectedRuntimeCfg.SubscriptionTimeout = DefaultSubscriptionTimeout
})

Context("With values in a viper config", func() {
Expand All @@ -66,12 +68,12 @@ var _ = Describe("Viper to Runtime Config", func() {
})
})

It("should only have 25 struct keys for tests to be valid", func() {
It("should only have 26 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(25))
Expect(keys).To(Equal(26))
})
})
5 changes: 4 additions & 1 deletion internal/runtime/defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,7 @@ package runtime

import "time"

var DefaultCSVTimeout = 180 * time.Second
var (
DefaultCSVTimeout = 180 * time.Second
DefaultSubscriptionTimeout = 180 * time.Second
)
18 changes: 14 additions & 4 deletions operator/check_operator.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,11 @@ const defaultScorecardWaitTime = "240"
// NewCheck is a check runner that executes the Operator Policy.
func NewCheck(image, indeximage string, kubeconfig []byte, opts ...Option) *operatorCheck {
c := &operatorCheck{
image: image,
kubeconfig: kubeconfig,
indeximage: indeximage,
scorecardWaitTime: defaultScorecardWaitTime,
image: image,
kubeconfig: kubeconfig,
indeximage: indeximage,
scorecardWaitTime: defaultScorecardWaitTime,
subscriptionTimeout: runtime.DefaultSubscriptionTimeout,
}

for _, opt := range opts {
Expand Down Expand Up @@ -94,6 +95,7 @@ func (c *operatorCheck) resolve(ctx context.Context) error {
Channel: c.operatorChannel,
Kubeconfig: c.kubeconfig,
CSVTimeout: c.csvTimeout,
SubscriptionTimeout: c.subscriptionTimeout,
})
if err != nil {
return fmt.Errorf("%w: %s", preflighterr.ErrCannotInitializeChecks, err)
Expand Down Expand Up @@ -174,6 +176,13 @@ func WithCSVTimeout(csvTimeout time.Duration) Option {
}
}

// WithSubscriptionTimeout customizes how long to wait for a subscription to become healthy.
func WithSubscriptionTimeout(subscriptionTimeout time.Duration) Option {
return func(oc *operatorCheck) {
oc.subscriptionTimeout = subscriptionTimeout
}
}

type operatorCheck struct {
// required
image string
Expand All @@ -191,4 +200,5 @@ type operatorCheck struct {
resolved bool
policy policy.Policy
csvTimeout time.Duration
subscriptionTimeout time.Duration
}

0 comments on commit 1cace8c

Please sign in to comment.