Setup registry.conf Env Vars for Buildah #21663

adambkaplan · 2018-12-14T18:56:06Z

Remove build-registry-conf mount
Supply ConfigMap for build-system-configs mount
Mount build-system-configs for all build containers
Rename registry config -> build system config

* Remove build-registry-conf mount * Supply ConfigMap for build-system-configs mount * Mount build-system-configs for all build containers * Rename registry config -> build system config JIRA-ID: [DEVEXP-154](https://jira.coreos.com/browse/DEVEXP-154)

adambkaplan

/assign @bparees
/cc @nalind

This is mostly a refactor. Per Nalin we were already prepared to read in registries.conf via the build-system-config mount.

No further changes to openshift/builder are necessary once this lands.

adambkaplan · 2018-12-14T18:59:58Z

pkg/build/controller/strategy/util.go

-	volumeName := apihelpers.GetName("build-system-configs", "build", kvalidation.DNS1123LabelMaxLength)
+func setupContainersConfigs(build *buildv1.Build, pod *corev1.Pod) {
+	const volumeName = "build-system-configs"
+	const configDir = ConfigMapBuildSystemConfigsMountPath


Set up registries.conf and potential future buildah configs for all containers in the build pod

adambkaplan · 2018-12-14T19:00:42Z

pkg/build/controller/strategy/util.go

+	// currently not created/managed by OpenShift
 	signaturePolicyPath := filepath.Join(configDir, "signature-policy.json")
+	// storage.conf configures storage policies for buildah
+	// currently not created/managed by OpenShift


doc the relevant paths that are passed in as buildah env vars.

adambkaplan · 2018-12-14T19:01:17Z

pkg/build/controller/strategy/util.go

 	}
 }
-
-func setupRegistries(build *buildv1.Build, pod *corev1.Pod) {


No longer needed - @nalind laid the groundwork in prior commits.

openshift-ci-robot · 2018-12-14T19:05:29Z

@adambkaplan: GitHub didn't allow me to request PR reviews from the following users: nalind.

Note that only openshift members and repo collaborators can review this PR, and authors cannot review their own PRs.

Details

In response to this:

/assign @bparees
/cc @nalind

This is mostly a refactor. Per Nalin we were already prepared to read in registries.conf via the build-system-config mount.

No further changes to openshift/builder are necessary once this lands.

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.

adambkaplan · 2018-12-14T20:15:04Z

/retest

centos mirror flake

bparees · 2018-12-14T20:32:40Z

/lgtm

but depending on what this does, you may be breaking some tests that rely on the default registry search behavior that comes from the registries.conf file embedded in the image. I couldn't tell if this PR is going to result in always mounting over that file w/ the configmap content, even if the user didn't supply config?

openshift-ci-robot · 2018-12-14T20:32:59Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: adambkaplan, bparees

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:

~~pkg/build/OWNERS~~ [bparees]

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

adambkaplan · 2018-12-14T21:01:02Z

@bparees this should no-op in the same fashion as #21653. We aren't overwriting the default registry config because we're mounting in a separate path. Per @nalind buildah will fall back to the image defaults if the registries.conf file does not exist.

openshift-bot · 2018-12-14T22:28:11Z

/retest