MGMT-7705: Use os_images in deploy/operator scripts #2708

danielerez · 2021-10-05T10:06:07Z

Assisted Pull Request

Description

To allow openshift_versions deprecation:

Use default_os_images instead of default_ocp_versions in common.sh and setup_assisted_operator.sh.
Removed unused logic in hack/get_ocp_versions_for_testing.sh.
handle_ocp_versions:
- Verify release images if specified.
- ocp_versions verification could be removed after merging: MGMT-7483: use release images defaults assisted-test-infra#1128

List all the issues related to this PR

What environments does this code impact?

Automation (CI, tools, etc)
Cloud
Operator Managed Deployments
None

How was this code tested?

assisted-test-infra environment
dev-scripts environment
Reviewer's test appreciated
Waiting for CI to do a full test run
Manual (Elaborate on how it was tested)
No tests needed

Assignees

/cc @YuviGold
/cc @filanov
/cc @osherdp

Checklist

Title and description added to both, commit and PR.
Relevant issues have been associated (see CONTRIBUTING guide)
Reviewers have been listed
This change does not require a documentation update (docstring, docs, README, etc)
Does this change include unit-tests (note that code changes require unit-tests)

Reviewers Checklist

Are the title and description (in both PR and commit) meaningful and clear?
Is there a bug required (and linked) for this change?
Should this PR be backported?

danielerez · 2021-10-05T11:59:37Z

/retest-required

danielerez · 2021-10-05T14:07:47Z

/retest-required

osherdp · 2021-10-05T14:22:50Z

tools/handle_ocp_versions.py


+    if release_images != None:


Suggested change

if release_images != None:

if release_images is not None:

osherdp · 2021-10-05T14:26:23Z

tools/handle_ocp_versions.py

-    if bool(args.single_version_only):
-        print("Using only single version of the RHCOS ISO", file=sys.stderr)
-        ocp_versions = update_openshift_versions_hashmap_to_single_only(ocp_versions, args.ocp_override)
+        verify_ocp_versions(ocp_versions, None)


Suggested change

verify_ocp_versions(ocp_versions, None)

verify_ocp_versions(ocp_versions, release_images=None)

osherdp · 2021-10-05T14:29:57Z

tools/deploy_assisted_installer_configmap.py

@@ -51,7 +51,7 @@ def get_deployment_tag(args):

 def main():
    utils.verify_build_directory(deploy_options.namespace)
-    verify_ocp_versions(json.loads(json.loads('"{}"'.format(deploy_options.ocp_versions))))
+    verify_ocp_versions(None, json.loads(json.loads('"{}"'.format(deploy_options.release_images))))


Suggested change

verify_ocp_versions(None, json.loads(json.loads('"{}"'.format(deploy_options.release_images))))

verify_ocp_versions(ocp_versions=None, release_images=json.loads(json.loads('"{}"'.format(deploy_options.release_images))))

osherdp · 2021-10-05T14:33:06Z

cmd/main.go

@@ -203,10 +203,11 @@ func main() {

 	var openshiftVersionsMap models.OpenshiftVersions
 	if Options.OpenshiftVersions == "" {
-		log.Fatal("OpenShift versions is empty")
+		log.Info("OpenShift versions is empty - using OsImages and ReleaseImages")


Suggested change

log.Info("OpenShift versions is empty - using OsImages and ReleaseImages")

log.Info("OpenShift versions is empty - using default values for OS and release images")

It's not necessarily the default values... as os_images/release_images can still be specified. But I'll rephrase to make it clearer.

Makefile

osherdp · 2021-10-05T14:37:50Z

Makefile

+ifndef RELEASE_IMAGES
+ifndef OS_IMAGES


what will happen if user overrides OS_IMAGES and not RELEASE_IMAGES?

That's the kube-api flow, i.e. specifying only OS_IMAGES. So we handle it in line 120.

deploy/operator/common.sh

YuviGold · 2021-10-06T13:24:51Z

internal/versions/versions.go

+	openshiftVersions := h.GetOpenshiftVersions()
+	if len(openshiftVersions) == 0 {
+		return errors.Errorf("No OS images are available")
+	}


Do we want to allow deploying the operator without any versions and add them in runtime?

So we can add only release images in runtime, the os images must exist on service start.

YuviGold · 2021-10-06T13:29:39Z

Makefile

+	RELEASE_IMAGES := $(shell (echo '$(DEFAULT_RELEASE_IMAGES)' | jq -c --arg v $(DEFAULT_VERSION) 'map(select(.openshift_version==$$v))'))
+	OS_IMAGES := $(shell (echo '$(DEFAULT_OS_IMAGES)' | jq -c --arg v $(DEFAULT_VERSION) 'map(select(.openshift_version==$$v))'))


This manipulation should only work in case CI is turned off (local deployments deploy a single image)
please comment a description of this behavior

right, added

YuviGold · 2021-10-06T13:31:02Z

Makefile

-# RHCOS_VERSION should be consistent with BaseObjectName in pkg/s3wrapper/client.go
-RHCOS_BASE_ISO := $(shell (jq -n '$(OPENSHIFT_VERSIONS)' | jq '[.[].rhcos_image]|max'))
+# Support all OS images if missing - for kube-api flow
+OS_IMAGES := $(or ${OS_IMAGES},${DEFAULT_OS_IMAGES})


It means that every deployment from test-infra will deploy all the versions, if I'm not mistaken (since test-infra overrides the environment variables)
So it needs to come up with a solution to a local test-infra (not CI) that deploys a single image to reduce deployment time

Isn't this line a duplication of line 107?

Nope, since line 107 is only for CI=true

YuviGold

LGTM

openshift-bot · 2021-10-06T17:57:09Z

/retest-required