CORS-2835: use build tags to produce installer with alternate infrastructure providers by patrickdillon · Pull Request #7656 · openshift/installer

patrickdillon · 2023-10-31T20:01:19Z

Build tags in the pkg/infrastructure/platform.go and pkg/infrastructure/platform_altinfra.go files selectively determine which file is included in the build. platform.go contains the normal Terraform dependencies for building infrastructure, while platform_altinfra.go will be where we land non-Terraform implementors of the provider interface.

By using the -v flag with go build we can see that no Terraform dependencies are included when building.

$ GOFLAGS='-mod=vendor -v' ./hack/build.sh &> normal-verbose-build.log
$ grep github.com/hashicorp/terraform/ normal-verbose-build.log | wc -l
72

$ go clean -cache

$ TAGS=altinfra GOFLAGS='-mod=vendor -v' ./hack/build.sh &> altinfra-verbose-build.log
$ grep github.com/hashicorp/terraform/ altinfra-verbose-build.log | wc -l
0

openshift/release#45169 will promote this build to a separate installer image.

openshift-ci-robot · 2023-10-31T20:01:22Z

@patrickdillon: This pull request references CORS-2876 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 story to target the "4.15.0" version, but no target version was set.

Details

In response to this:

Adds an initial Dockerfile and uses build tags to create a Terraform-free installer-altinfra image.

TODO: requires followup work in openshift/release to promote the 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.

openshift-ci · 2023-10-31T20:01:28Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

openshift-ci-robot · 2023-11-01T17:21:55Z

@patrickdillon: This pull request references CORS-2876 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 story to target the "4.15.0" version, but no target version was set.

Details

In response to this:

Adds an initial Dockerfile and uses build tags to create a Terraform-free installer-altinfra 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.

Return an error rather than panic for unsupported platforms.

Implements a build tag strategy to allow producing installer binaries using an alternate (to terraform) infrastructure provider. Using build tags we can select different ProviderForPlatform implementations. Because the altinfra implementation of that function does not depend on any terraform dependencies, none are included in the binary.

When building the altinfra or ARO images, no terraform dependencies should be involved, so skip them in the build script.

patrickdillon · 2023-11-01T20:14:52Z

/retest

openshift-ci-robot · 2023-11-02T01:08:11Z

@patrickdillon: This pull request references CORS-2876 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 story to target the "4.15.0" version, but no target version was set.

Details

In response to this:

Adds an initial Dockerfile and uses build tags to create a Terraform-free installer-altinfra image.

Build Tags

Build tags in the pkg/infrastructure/platform.go and pkg/infrastructure/platform_altinfra.go files selectively determine which file is included in the build. platform.go contains the normal Terraform dependencies for building infrastructure, while platform_altinfra.go will be where we land non-Terraform implementors of the provider interface.

By using the -v flag with go build we can see that no Terraform dependencies are included when building.
$ GOFLAGS='-mod=vendor -v' ./hack/build.sh &> normal-verbose-build.log
$ grep github.com/hashicorp/terraform/ normal-verbose-build.log | wc -l
72

$ go clean -cache

$ TAGS=altinfra GOFLAGS='-mod=vendor -v' ./hack/build.sh &> altinfra-verbose-build.log
$ grep github.com/hashicorp/terraform/ altinfra-verbose-build.log | wc -l
0

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.

openshift-ci-robot · 2023-11-02T01:51:48Z

@patrickdillon: This pull request references CORS-2876 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 story to target the "4.15.0" version, but no target version was set.

Details

In response to this:

Adds an initial Dockerfile and uses build tags to create a Terraform-free installer-altinfra image.

Build Tags

Build tags in the pkg/infrastructure/platform.go and pkg/infrastructure/platform_altinfra.go files selectively determine which file is included in the build. platform.go contains the normal Terraform dependencies for building infrastructure, while platform_altinfra.go will be where we land non-Terraform implementors of the provider interface.

By using the -v flag with go build we can see that no Terraform dependencies are included when building.
$ GOFLAGS='-mod=vendor -v' ./hack/build.sh &> normal-verbose-build.log
$ grep github.com/hashicorp/terraform/ normal-verbose-build.log | wc -l
72

$ go clean -cache

$ TAGS=altinfra GOFLAGS='-mod=vendor -v' ./hack/build.sh &> altinfra-verbose-build.log
$ grep github.com/hashicorp/terraform/ altinfra-verbose-build.log | wc -l
0
Dockerfile

Dockerfile builds with the altinfra build tag. This Dockerfile will be used by openshift/release#45169 to promote and include this image in the release payload.

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.

patrickdillon · 2023-11-02T14:26:28Z

Rafael pointed out that we don't need the dockerfile. The only difference is the tag, which can be controlled in the ci job. Updating.

openshift-ci-robot · 2023-11-02T14:30:56Z

@patrickdillon: This pull request references CORS-2835 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 story to target the "4.15.0" version, but no target version was set.

Details

In response to this:

Adds an initial Dockerfile and uses build tags to create a Terraform-free installer-altinfra image.

Build Tags

Build tags in the pkg/infrastructure/platform.go and pkg/infrastructure/platform_altinfra.go files selectively determine which file is included in the build. platform.go contains the normal Terraform dependencies for building infrastructure, while platform_altinfra.go will be where we land non-Terraform implementors of the provider interface.

By using the -v flag with go build we can see that no Terraform dependencies are included when building.
$ GOFLAGS='-mod=vendor -v' ./hack/build.sh &> normal-verbose-build.log
$ grep github.com/hashicorp/terraform/ normal-verbose-build.log | wc -l
72

$ go clean -cache

$ TAGS=altinfra GOFLAGS='-mod=vendor -v' ./hack/build.sh &> altinfra-verbose-build.log
$ grep github.com/hashicorp/terraform/ altinfra-verbose-build.log | wc -l
0
Dockerfile

Dockerfile builds with the altinfra build tag. This Dockerfile will be used by openshift/release#45169 to promote and include this image in the release payload.

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.

openshift-ci-robot · 2023-11-02T14:31:45Z

@patrickdillon: This pull request references CORS-2835 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 story to target the "4.15.0" version, but no target version was set.

Details

In response to this:

Build tags in the pkg/infrastructure/platform.go and pkg/infrastructure/platform_altinfra.go files selectively determine which file is included in the build. platform.go contains the normal Terraform dependencies for building infrastructure, while platform_altinfra.go will be where we land non-Terraform implementors of the provider interface.

By using the -v flag with go build we can see that no Terraform dependencies are included when building.
$ GOFLAGS='-mod=vendor -v' ./hack/build.sh &> normal-verbose-build.log
$ grep github.com/hashicorp/terraform/ normal-verbose-build.log | wc -l
72

$ go clean -cache

$ TAGS=altinfra GOFLAGS='-mod=vendor -v' ./hack/build.sh &> altinfra-verbose-build.log
$ grep github.com/hashicorp/terraform/ altinfra-verbose-build.log | wc -l
0
openshift/release#45169 will promote this build to a separate installer 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.

patrickdillon · 2023-11-02T14:47:44Z

Dropped the Dockerfile commit

andfasano · 2023-11-02T17:36:10Z

pkg/infrastructure/platform/platform.go

+		return terraform.InitializeProvider([]terraform.Stage{}), nil
 	}
-	panic(fmt.Sprintf("unsupported platform %q", platform))
+	return nil, fmt.Errorf("unsupported platform %q", platform)


nit: since essentially the terraform.InitializeProvider is always invoked for each platform stage type, code could be slightly simplified by storing the relationship in a map, ie:

platformStages := map[string][]terraform.Stage{ alibabacloudtypes.Name: alibabacloud.PlatformStages, awstypes.Name: aws.PlatformStages, // ... other types } s, ok := platformStages[platform] if !ok { return nil, fmt.Errorf("unsupported platform %q", platform) } return terraform.InitializeProvider(s)

This is a good suggestion. I would like to keep it this way, because once #7413 merges, we can introduce alternate providers with feature gates in these case statements, so we would have something like:

case awstypes.Name: if fg.Enabled(configv1.FeatureGateInstallAWSSDK) { return aws.InitializeProvider("") } return terraform.InitializeProvider(aws.PlatformStages), nil

Ok, we could eventually review it in future

andfasano · 2023-11-02T17:40:00Z

hack/build.sh


 # Build terraform binaries before setting environment variables since it messes up make
-if test "${SKIP_TERRAFORM}" != y
+if test "${SKIP_TERRAFORM}" != y && ! (echo "${TAGS}" | grep -q -e 'aro' -e 'altinfra')


I wasn't aware about the specific build tag aro, must be skipped as well?

Yes, so I have also added the aro tag here:
https://github.com/openshift/installer/pull/7656/files#diff-a4f0f6b61545d617e8db370db204ea8d9fbd62f380527ef49351bf617c781e32R1-R2

ARO has a fork of the installer and does not use Terraform (using ARO-RP instead). I believe they use this ARO build tag when building their fork, so the result should be that the ARO version of the installer is built without Terraform dependencies.

andfasano · 2023-11-06T17:35:47Z

/approve

openshift-ci · 2023-11-06T17:36:14Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: andfasano

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~~ [andfasano]

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

r4f4

/lgtm

openshift-ci-robot · 2023-11-07T14:25:24Z

/retest-required

Remaining retests: 0 against base HEAD 53ce73d and 2 for PR HEAD 7014647 in total

patrickdillon · 2023-11-07T15:23:15Z

/skip

openshift-ci · 2023-11-07T15:48:11Z

@patrickdillon: all tests passed!

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.

openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Oct 31, 2023

openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 31, 2023

patrickdillon marked this pull request as ready for review November 1, 2023 17:21

openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 1, 2023

openshift-ci bot requested review from pawanpinjarkar and rna-afk November 1, 2023 17:22

patrickdillon mentioned this pull request Nov 1, 2023

Installer: promote altinfra image openshift/release#45169

Merged

patrickdillon added 3 commits November 1, 2023 14:46

pkg/infrastructure: errorr on unsupported platform

eb1d8ef

Return an error rather than panic for unsupported platforms.

hack/build.sh: skip terraform for altinfra or aro

7014647

When building the altinfra or ARO images, no terraform dependencies should be involved, so skip them in the build script.

patrickdillon force-pushed the altinfra-image-tags branch from ecdd91b to 3492c4a Compare November 1, 2023 18:49

patrickdillon force-pushed the altinfra-image-tags branch from 3492c4a to 7014647 Compare November 2, 2023 14:28

patrickdillon changed the title ~~CORS-2835, CORS-2876: create terraform-free altinfra image~~ CORS-2835: use build tags to produce installer with alternate infrastructure providers Nov 2, 2023

andfasano reviewed Nov 2, 2023

View reviewed changes

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 6, 2023

jcpowermac mentioned this pull request Nov 6, 2023

SPLAT-1213: vSphere implement infra provision via sdk #7626

Closed

r4f4 reviewed Nov 7, 2023

View reviewed changes

openshift-ci bot assigned r4f4 Nov 7, 2023

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Nov 7, 2023

openshift-merge-bot bot merged commit 2596e5d into openshift:master Nov 7, 2023

Conversation

patrickdillon commented Oct 31, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci-robot commented Oct 31, 2023 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci bot commented Oct 31, 2023

Uh oh!

openshift-ci-robot commented Nov 1, 2023 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

patrickdillon commented Nov 1, 2023

Uh oh!

openshift-ci-robot commented Nov 2, 2023 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci-robot commented Nov 2, 2023 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

patrickdillon commented Nov 2, 2023

Uh oh!

openshift-ci-robot commented Nov 2, 2023 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci-robot commented Nov 2, 2023 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

patrickdillon commented Nov 2, 2023

Uh oh!

andfasano Nov 2, 2023

Choose a reason for hiding this comment

Uh oh!

patrickdillon Nov 3, 2023

Choose a reason for hiding this comment

Uh oh!

andfasano Nov 6, 2023

Choose a reason for hiding this comment

Uh oh!

andfasano Nov 2, 2023

Choose a reason for hiding this comment

Uh oh!

patrickdillon Nov 3, 2023

Choose a reason for hiding this comment

Uh oh!

andfasano commented Nov 6, 2023

Uh oh!

openshift-ci bot commented Nov 6, 2023

Uh oh!

r4f4 left a comment

Choose a reason for hiding this comment

Uh oh!

openshift-ci-robot commented Nov 7, 2023

Uh oh!

patrickdillon commented Nov 7, 2023

Uh oh!

openshift-ci bot commented Nov 7, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Comments

patrickdillon commented Oct 31, 2023 •

edited

Loading

openshift-ci-robot commented Oct 31, 2023 •

edited by openshift-ci bot

Loading

openshift-ci-robot commented Nov 1, 2023 •

edited by openshift-ci bot

Loading

openshift-ci-robot commented Nov 2, 2023 •

edited by openshift-ci bot

Loading

openshift-ci-robot commented Nov 2, 2023 •

edited by openshift-ci bot

Loading

openshift-ci-robot commented Nov 2, 2023 •

edited by openshift-ci bot

Loading

openshift-ci-robot commented Nov 2, 2023 •

edited by openshift-ci bot

Loading