Skip to content
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
70 changes: 47 additions & 23 deletions cmd/deploy.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,20 @@ this flag can be used to tell roxie how to pre-load images for the current clust
}),
)

registerFlag(cmd, settings, "central-tag", "Image tag for Central (overrides --tag for Central)",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need to think a bit about this PR from a conceptual viewpoint. But, looking at the concrete changes, I can already provide limited feedback.
For example, I don't think we need these flags. Direct CLI flags (IMHO) are remnants from the time before we had the unified roxie config. Everything can be set there and these settings specifically are not settings that we need every day (outside of automation), hence I think it's perfectly fine to drop these and let users configure those in the roxie config if they need to.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a roxie user I'd actually like to have them.

To give you an example, for cert refresh and CA rotation I had to do a lot of mixed version testing. It was quite tedious to do and because of that I didn't do as much compatibility testing as I should have. And these flags would make it quite easy to be able to switch around versions easily.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If they are important for you, fine.

But you do know that you can do the same with --set, right?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, didn't think about that. Then I'm inclined to agree with you.

Having a flag is still somewhat useful because it makes the feature more easily discoverable, that's the only argument I have for it now.

withApplyFn("version", func(config *deployer.Config, tag string) error {
config.Central.Operator.Version = tag
return nil
}),
)

registerFlag(cmd, settings, "secured-cluster-tag", "Image tag for SecuredCluster (overrides --tag for SecuredCluster)",
withApplyFn("version", func(config *deployer.Config, tag string) error {
config.SecuredCluster.Operator.Version = tag
return nil
}),
)

registerFlag(cmd, settings, "operator-env", "Operator environment variables (e.g., RELATED_IMAGE_MAIN=quay.io/...)",
withApplyFn("env-var", func(config *deployer.Config, envExpr string) error {
key, value, err := deployer.ParseOperatorEnvVar(envExpr)
Expand Down Expand Up @@ -241,25 +255,6 @@ func runDeploy(cmd *cobra.Command, args []string) error {
return err
}

if !deploySettings.Central.EarlyReadinessEnabled() || !deploySettings.SecuredCluster.EarlyReadinessEnabled() {
// Explanation on the versions involved here:
// Deploying StackRox begins with picking a "main image tag" -- this is a version identifier, which cannot be reliably parsed as a semver.
// But there is a derived version from that -- the operator version -- which can be parsed as a semver.
//
// The invocation of deploySettings.Operator.Configure() above in this function prepares the operator deployment config in the sense
// that top-level roxie configuration options are propagated to the concrete operator deployment configuration. This includes also
// storing of the derived operator version within the operator configuration.
//
// This is why we use the operator version here when checking version constraints.
hasSupport, err := stackroxversions.SupportsAdditionalPrinterColumns(deploySettings.Operator.Version)
if err != nil {
return fmt.Errorf("checking version constraint on main image tag %s: %w", deploySettings.Roxie.Version, err)
}
if !hasSupport {
return fmt.Errorf("--early-readiness=false can only be used for StackRox versions satisfying %s", stackroxversions.SupportsAdditionalPrinterColumnsConstraint.String())
}
}

d, err := deployer.New(log)
if err != nil {
return fmt.Errorf("failed to create deployer: %w", err)
Expand Down Expand Up @@ -390,10 +385,6 @@ func configureConfig(log *logger.Logger, components component.Component, deployS
return fmt.Errorf("configuring operator configuration: %w", err)
}

if deploySettings.Roxie.KonfluxImagesEnabled() {
deployer.PopulateKonfluxEnvVars(deploySettings)
}

if components.IncludesCentral() {
if err := deploySettings.Central.ConfigureSpec(&deploySettings.Roxie); err != nil {
return fmt.Errorf("configuring Central spec: %w", err)
Expand Down Expand Up @@ -458,5 +449,38 @@ func deployValidate(components component.Component, deploySettings *deployer.Con
}
}

if deploySettings.HasMixedVersions() {
globalLogger.Dimf("Mixed versions detected (configured via --central-tag / --secured-cluster-tag or central.operator / securedCluster.operator)")
if components.IncludesOperatorExplicitly() {
return errors.New("mixed versions are not supported with operator-only deploy")
}
if deploySettings.Operator.DeployViaOlmEnabled() {
return errors.New("mixed versions are not supported with OLM deployment mode")
}
}

if !deploySettings.Central.EarlyReadinessEnabled() || !deploySettings.SecuredCluster.EarlyReadinessEnabled() {
// Explanation on the versions involved here:
// Deploying StackRox begins with picking a "main image tag" -- this is a version identifier, which cannot be reliably parsed as a semver.
// But there is a derived version from that -- the operator version -- which can be parsed as a semver.
//
// The invocation of deploySettings.Operator.Configure() above in this function prepares the operator deployment config in the sense
// that top-level roxie configuration options are propagated to the concrete operator deployment configuration. This includes also
// storing of the derived operator version within the operator configuration.
//
// This is why we use the operator version here when checking version constraints.
// Check every operator instance that will be deployed.
for _, instance := range deploySettings.OperatorInstances() {
hasSupport, err := stackroxversions.SupportsAdditionalPrinterColumns(instance.Version)
if err != nil {
return fmt.Errorf("checking version constraint on operator version %s: %w", instance.Version, err)
}
if !hasSupport {
return fmt.Errorf("--early-readiness=false can only be used for StackRox versions satisfying %s (operator version %s in %s does not)",
stackroxversions.SupportsAdditionalPrinterColumnsConstraint.String(), instance.Version, instance.Namespace)
}
}
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

return nil
}
45 changes: 33 additions & 12 deletions internal/deployer/acs_images.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,30 +7,51 @@ import (
)

func imagesForConfig(config Config) []string {
images := make([]string, 0)
var images []string
prefix := ""
if config.Roxie.KonfluxImagesEnabled() {
prefix = "release-"
}

imageRegistry := constants.DefaultRegistry
images = append(images, fmt.Sprintf("%s/%s%s:%s", imageRegistry, prefix, "main", config.Roxie.Version))
images = append(images, fmt.Sprintf("%s/%s%s:%s", imageRegistry, prefix, "central-db", config.Roxie.Version))
images = append(images, fmt.Sprintf("%s/%s%s:%s", imageRegistry, prefix, "scanner-v4-db", config.Roxie.Version))
images = append(images, fmt.Sprintf("%s/%s%s:%s", imageRegistry, prefix, "scanner-v4", config.Roxie.Version))

for _, mainTag := range uniqueMainVersions(config) {
images = append(images,
fmt.Sprintf("%s/%s%s:%s", imageRegistry, prefix, "main", mainTag),
fmt.Sprintf("%s/%s%s:%s", imageRegistry, prefix, "central-db", mainTag),
fmt.Sprintf("%s/%s%s:%s", imageRegistry, prefix, "scanner-v4-db", mainTag),
fmt.Sprintf("%s/%s%s:%s", imageRegistry, prefix, "scanner-v4", mainTag),
)
}

operatorPrefix := prefix
if !config.Roxie.KonfluxImagesEnabled() {
prefix = "stackrox-"
operatorPrefix = "stackrox-"
}
for _, instance := range config.OperatorInstances() {
images = append(images,
fmt.Sprintf("%s/%s%s:%s", imageRegistry, operatorPrefix, "operator", instance.Version),
OperatorBundleImage(instance.Version, config.Roxie.KonfluxImagesEnabled()),
)
}
images = append(images, fmt.Sprintf("%s/%s%s:%s", imageRegistry, prefix, "operator", config.Operator.Version))
images = append(images, OperatorBundleImage(config))

return images
}

func OperatorBundleImage(config Config) string {
func uniqueMainVersions(config Config) []string {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My feeling is that this can be simplified.
For example, don't Effect*Version() already handle the Roxie.Version fallback?
In that case, wouldn't a single slices.Compact() on the versions slice be sufficient?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 9169418

central := config.CentralVersion()
sc := config.SecuredClusterVersion()
if central == sc {
return []string{central}
}
return []string{central, sc}
}

// OperatorBundleImage returns the operator bundle image for a specific operator version.
func OperatorBundleImage(operatorVersion string, konflux bool) string {
imageRegistry := constants.DefaultRegistry
if config.Roxie.KonfluxImagesEnabled() {
return fmt.Sprintf("%s/release-operator-bundle:v%s", imageRegistry, config.Operator.Version)
if konflux {
return fmt.Sprintf("%s/release-operator-bundle:v%s", imageRegistry, operatorVersion)
}
return fmt.Sprintf("%s/stackrox-operator-bundle:v%s", imageRegistry, config.Operator.Version)
return fmt.Sprintf("%s/stackrox-operator-bundle:v%s", imageRegistry, operatorVersion)
}
16 changes: 11 additions & 5 deletions internal/deployer/config.go
Comment thread
mclasmeier marked this conversation as resolved.
Original file line number Diff line number Diff line change
Expand Up @@ -77,10 +77,12 @@ func NewRoxieConfig() RoxieConfig {

// OperatorConfig controls how the ACS operator is deployed.
type OperatorConfig struct {
SkipDeployment *bool `yaml:"skipDeployment,omitempty"`
DeployViaOlm *bool `yaml:"deployViaOlm,omitempty"`
Version string `yaml:"version,omitempty"`
EnvVars map[string]string `yaml:"envVars,omitempty"`
SkipDeployment *bool `yaml:"skipDeployment,omitempty"`
DeployViaOlm *bool `yaml:"deployViaOlm,omitempty"`
// Version can hold either a main image tag or an operator tag. It should be normalized to an
// operator tag via ConvertToOperatorTag before use (the function is idempotent, so either form is accepted).
Version string `yaml:"version,omitempty"`
EnvVars map[string]string `yaml:"envVars,omitempty"`
}

func (c *OperatorConfig) SkipDeploymentSet() bool {
Expand All @@ -101,7 +103,7 @@ func (c *OperatorConfig) DeployViaOlmEnabled() bool {

// Configure derives the operator version from the roxie configuration.
func (c *OperatorConfig) Configure(roxieConfig *RoxieConfig) error {
c.Version = helpers.ConvertMainTagToOperatorTag(roxieConfig.Version)
c.Version = helpers.ConvertToOperatorTag(roxieConfig.Version)
return nil
}

Expand All @@ -115,6 +117,8 @@ type WaitConfig struct {

// CentralConfig holds deployment settings for the Central component.
type CentralConfig struct {
// Operator allows per-component operator overrides; only used in dual-operator mode.
Operator OperatorConfig `yaml:"operator,omitempty"`
Namespace string `yaml:"namespace,omitempty"`
ResourceProfile types.ResourceProfile `yaml:"resourceProfile,omitempty"`
PauseReconciliation *bool `yaml:"pauseReconciliation,omitempty"`
Expand Down Expand Up @@ -262,6 +266,8 @@ func (c *CentralConfig) CustomResource() (map[string]interface{}, error) {

// SecuredClusterConfig holds deployment settings for the SecuredCluster component.
type SecuredClusterConfig struct {
// Operator allows per-component operator overrides; only used in dual-operator mode.
Operator OperatorConfig `yaml:"operator,omitempty"`
Namespace string `yaml:"namespace,omitempty"`
ResourceProfile types.ResourceProfile `yaml:"resourceProfile,omitempty"`
PauseReconciliation *bool `yaml:"pauseReconciliation,omitempty"`
Expand Down
Loading
Loading