CORS-2785: images: decouple installer and installer-artifacts

[r4f4]

The installer-artifacts image depends on the installer image, which means they cannot be built in parallel. The effect of this dependency is that the installer-artifacts image contains the linux amd64 builds in addition to the mac/linux amd64/arm64 builds present in the installer-artifacts image.

Breaking the dependency results in a more efficient build process because they can be built in parallel as well as no duplication of the linux amd64 binary in the release image (before this change the binary would be present in both images within the release image).

The risk, though, is that clients are inconsistently grabbing the linux amd64 binary from either image, rather than only the installer image, in which case we would break any client trying to get that binary from the installer-artifacts image.

[openshift-ci-robot]

@r4f4: This pull request references CORS-2785 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the spike to target the "4.15.0" version, but no target version was set.

Details

In response to this:

The installer-artifacts image depends on the installer image, which means they cannot be built in parallel. The effect of this dependency is that the installer-artifacts image contains the linux amd64 builds in addition to the mac/linux amd64/arm64 builds present in the installer-artifacts image.

Breaking the dependency results in a more efficient build process because they can be built in parallel as well as no duplication of the linux amd64 binary in the release image (before this change the binary would be present in both images within the release image).

The risk, though, is that clients are inconsistently grabbing the linux amd64 binary from either image, rather than only the installer image, in which case we would break any client trying to get that binary from the installer-artifacts image.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[r4f4]

Reopening #7159 with commits squashed and conflicts resolved.

[r4f4]

/assign @patrickdillon

[r4f4]

/test verify-vendor
/test verify-codegen
/test yaml-lint
/test unit
/test tf-lint
/test tf-fmt
/test govet
/test golint

[r4f4]

/test okd-unit
/test okd-verify-codegen
/test okd-scos-unit
/test okd-scos-images
/test okd-images

[r4f4]

/test okd-scos-verify-codegen

[patrickdillon]

It looks like we would need to update oc to extract the linux amd64 binary from the installer image:

https://github.com/openshift/oc/blob/d76174f58c86b2ad27f99f3ebdb3eaa5c945256a/pkg/cli/admin/release/extract_tools.go#L327-L338

[r4f4]

It looks like we would need to update oc to extract the linux amd64 binary from the installer image:

https://github.com/openshift/oc/blob/d76174f58c86b2ad27f99f3ebdb3eaa5c945256a/pkg/cli/admin/release/extract_tools.go#L327-L338

@patrickdillon I thought you had already determined that oc is fine? #7159 (comment)

[r4f4]

@patrickdillon What about openshift/oc#1608 ?

[patrickdillon]

It looks like we would need to update oc to extract the linux amd64 binary from the installer image:

https://github.com/openshift/oc/blob/d76174f58c86b2ad27f99f3ebdb3eaa5c945256a/pkg/cli/admin/release/extract_tools.go#L327-L338

It looks like we would need to update oc to extract the linux amd64 binary from the installer image:
https://github.com/openshift/oc/blob/d76174f58c86b2ad27f99f3ebdb3eaa5c945256a/pkg/cli/admin/release/extract_tools.go#L327-L338

@patrickdillon I thought you had already determined that oc is fine? #7159 (comment)

After rereading I see that my initial comments misunderstood what was happening as a result of this PR. I misunderstood and thought that we would only have linux amd64 in the installer image, which is incorrect: linux amd64 will still be in installer-artifacts as well.

@patrickdillon What about openshift/oc#1608 ?

I believe my original comment was based on a misunderstanding so do we still need it? If we don't, I'd rather leave it alone.

Sorry for the confusion.

[r4f4]

@patrickdillon What about openshift/oc#1608 ?

I believe my original comment was based on a misunderstanding so do we still need it? If we don't, I'd rather leave it alone.

I think with that PR we can build only installer-artifacts for the release payload, no need for installer. At least I fail to see for what else would we need installer.

[patrickdillon]

@patrickdillon What about openshift/oc#1608 ?

I believe my original comment was based on a misunderstanding so do we still need it? If we don't, I'd rather leave it alone.

I think with that PR we can build only installer-artifacts for the release payload, no need for installer. At least I fail to see for what else would we need installer.

Interesting idea. I think we need these coreos manifests as well, but that would be it: https://github.com/openshift/installer/blob/master/images/installer/Dockerfile.ci#L9

[patrickdillon]

Well, and we would need to update whatever clients depend on those coreos manifests

[r4f4]

@patrickdillon What about openshift/oc#1608 ?

I believe my original comment was based on a misunderstanding so do we still need it? If we don't, I'd rather leave it alone.

I think with that PR we can build only installer-artifacts for the release payload, no need for installer. At least I fail to see for what else would we need installer.

Interesting idea. I think we need these coreos manifests as well, but that would be it: https://github.com/openshift/installer/blob/master/images/installer/Dockerfile.ci#L9

I've added bdd3964 so now installer-artifacts also contains the manifests. That way we can build artifacts independently without breaking any workflow looking for them.

[r4f4]

@patrickdillon I've decided to drop the idea of changing oc for now. I think just not having installer-artifacts depend on the installer image is enough for now. It shouldn't break anything since oc will still be able to grab the openshift-installer binary either from the installer-artifacts or the installer image. The only difference is that the images can be built in parallel both in CI and during release. WDYT?

[r4f4]

/test verify-vendor
/test verify-codegen
/test unit
/test images
/test yaml-lint
/test shellcheck
/test tf-lint
/test golint
/test gofmt

[r4f4]

/test okd-images
/test okd-scos-images
/test okd-verify-codegen

[patrickdillon]

@patrickdillon I've decided to drop the idea of changing oc for now. I think just not having installer-artifacts depend on the installer image is enough for now. It shouldn't break anything since oc will still be able to grab the openshift-installer binary either from the installer-artifacts or the installer image. The only difference is that the images can be built in parallel both in CI and during release. WDYT?

I think this is the right call. 👍

[rna-afk]

/lgtm

[patrickdillon]

I've added bdd3964 so now installer-artifacts also contains the manifests. That way we can build artifacts independently without breaking any workflow looking for them.

Did you drop this commit from the PR?

IIUC, prior to this PR the coreos manifests are included in installer-artifacts because we are using installer as a base image. With the changes in this PR, the installer-artifacts PR would no longer include those manifests.

I'm not sure exactly how the manifests are used, but I think it's safer to continue to include them in the image.

[r4f4]

I've added bdd3964 so now installer-artifacts also contains the manifests. That way we can build artifacts independently without breaking any workflow looking for them.

Did you drop this commit from the PR?

IIUC, prior to this PR the coreos manifests are included in installer-artifacts because we are using installer as a base image. With the changes in this PR, the installer-artifacts PR would no longer include those manifests.

I'm not sure exactly how the manifests are used, but I think it's safer to continue to include them in the image.

@patrickdillon I somehow misunderstood the manifests were being consumed from the installer image. I've brought the commit back.

[patrickdillon]

/approve
/lgtm

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: patrickdillon

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [patrickdillon]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[openshift-ci]

@r4f4: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/okd-e2e-aws-ovn-upgrade	f5c4035	link	false	/test okd-e2e-aws-ovn-upgrade

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.