Skip to content

ROX-9249: Allow override Central DB image#1030

Merged
c-du merged 7 commits intomasterfrom
cong/image-sel
Mar 24, 2022
Merged

ROX-9249: Allow override Central DB image#1030
c-du merged 7 commits intomasterfrom
cong/image-sel

Conversation

@c-du
Copy link
Contributor

@c-du c-du commented Mar 22, 2022

Description

We cannot choose Central DB image as of now. It is derived from main image.
This PR allows select Central DB image separately from Main.

Checklist

  • Investigated and inspected CI test results
  • Unit test and regression tests added
  • [ ] Evaluated and added CHANGELOG entry if required
  • [ ] Determined and documented upgrade steps
  • [ ] Documented user facing changes (create PR based on [stackrox/openshift-docs](https://github.com/stackrox/openshift-docs) and merge into rhacs-docs)

If any of these don't apply, please comment below.

Testing Performed

Run CI tests and deploy with postgres flag enabled.

TODO(replace-me)
Use this space to explain how you tested your PR, or, if you didn't test it, why
you did not do so. Valid reasons include, for example, "CI is sufficient",
"No testable changes". Feel free to attach JSON snippets, curl commands,
screenshots.

In addition to reviewing your code, reviewers must also review your testing
instructions and make sure they are sufficient.

@ghost
Copy link

ghost commented Mar 22, 2022

Tag for build #346573 is 3.69.x-176-g269e9df8af.

💻 For deploying this image using the dev scripts, run the following first:

export MAIN_IMAGE_TAG='3.69.x-176-g269e9df8af'

📦 You can also generate an installation bundle with:

docker run -i --rm stackrox/main:3.69.x-176-g269e9df8af central generate interactive > bundle.zip

🕹️ A roxctl binary artifact can be downloaded from CircleCI.

@c-du c-du requested a review from connorgorman March 22, 2022 22:39
@c-du c-du marked this pull request as ready for review March 22, 2022 22:39
@c-du c-du requested review from a team as code owners March 22, 2022 22:39
Copy link
Contributor

@janisz janisz left a comment

Choose a reason for hiding this comment

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

Please add description for this PR

Copy link
Contributor

@vikin91 vikin91 left a comment

Choose a reason for hiding this comment

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

The changes in this PR look fine, but I it is difficult to say whether there they are complete (maybe there should be more changes for this?)

I would be interested how this has been tested. I know this is feature-flagged, but adding bats and expect tests for roxctl could really help catching potential problems.

@vikin91
Copy link
Contributor

vikin91 commented Mar 23, 2022

@fredrb could you maybe also give it a short look? You have the most experience with this area

Copy link
Contributor

@fredrb fredrb left a comment

Choose a reason for hiding this comment

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

My biggest concern is releasing this without having the downstream image available. If it was already created then you can ignore my inline comments.

MainImageName: "rhacs-main-rhel8",
MainImageTag: v.MainVersion,
CentralDBImageTag: v.MainVersion,
CentralDBImageName: "rhacs-central-db-rhel8",
Copy link
Contributor

Choose a reason for hiding this comment

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

Just keep in mind that this image has to be built downstream (CPaaS). I assume this was not done yet.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I will put a TODO tag with jira blocker.

initContainers:
- name: init-db
image: {{ ._rox.central.image.fullRef | replace "main" "central-db" | quote }}
image: {{ ._rox.central.dbImage.fullRef | quote }}
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't we wrap this with a meta template feature flag check? At least until the image is available downstream. If this is released prior to rhacs-central-db-rhel8 being available, the init container will crash when on rhacs flavor.

The meta template would look something like this:

[<- if .FeatureFlags.FLAG_NAME >]
...
[<- else >]
...
[<- end >]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is it possible to release upstream only? We use enableCentralDB flag, which will block this template file entirely. See line #2.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh ok. That makes sense. I missed the conditional check on line 2.

Is it possible to release upstream only?

I'm not sure if it is. Technically the downstream release (registry.redhat.io) is our official release. I think it should be fine as long as:

  1. This file isn't included if enableCentralDB is not enabled (which is covered by line 2).
  2. We create a rhacs-central-db-rhel8 image downstream once this needs to be released (covered by the task you created ROX-9858).

Copy link
Contributor

Choose a reason for hiding this comment

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

If possible, please update the JIRA description with a pointer to this PR and mention that the image that needs to be written is for rhacs-central-db-rhel8.

@c-du c-du requested a review from fredrb March 23, 2022 17:51
@c-du c-du requested a review from vikin91 March 24, 2022 15:37
@c-du c-du merged commit de3cc7c into master Mar 24, 2022
@c-du c-du deleted the cong/image-sel branch March 24, 2022 20:59
Comment on lines +45 to +69
if {[info exists ::env(ROX_POSTGRES_DATASTORE)] && [string equal "$env(ROX_POSTGRES_DATASTORE)" true]} {
# Enter central-db image to use (default: "docker.io/stackrox/central-db:2.21.0-15-g448f2dc8fa"):
# Enter central-db image to use (default: "stackrox.io/central-db:3.67.x-296-g56df6a892d"):
# Enter central-db image to use (default: "registry.redhat.io/advanced-cluster-security/rhacs-central-db-rhel8:3.68.x-30-g516b4e7a6c-dirty"):
expect {
default {
send_user "\nFATAL: No question about Central-DB image\n"
exit 8
}
"Enter central-db * (if unset, the default will be used):" {
send_user "WARNING: roxctl does not suggest any registry for central-db"
send "\n"
set exitWith [expr {$exitWith + 2}]
}
"Enter central-db * (default: \"$registry/central-db:*\"):" {
send_user "roxctl suggests correct registry for central-db"
send "\n"
}
# Special case for RHACS to avoid writing a regexp in TCL
"Enter central-db * (default: \"$registry/rhacs-central-db-rhel8:*\"):" {
send_user "roxctl suggests correct registry for central-db"
send "\n"
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Feature-flags in expect - cool!

RTann pushed a commit that referenced this pull request Apr 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants