BUILD-281: wire in OCM feature gate config fields to allow eventual use of CSI volumes when BUILD-275 lands by gabemontero · Pull Request #199 · openshift/openshift-controller-manager

gabemontero · 2021-09-10T17:33:44Z

No description provided.

gabemontero · 2021-09-10T17:34:32Z

 	k8s.io/utils v0.0.0-20210707171843-4b05e18ac7d9
 )

+replace github.com/openshift/api => github.com/gabemontero/api v0.0.0-20210910154316-eaa29e1f16df


this captures the latest state of openshift/api#1007

when that merges, we'll just change the version for openshift/api up above

gabemontero · 2021-09-10T17:34:52Z


+replace github.com/openshift/api => github.com/gabemontero/api v0.0.0-20210910154316-eaa29e1f16df
+
+replace github.com/openshift/library-go => github.com/gabemontero/library-go v0.0.0-20210910154638-8565ba189275


this captures the latest state of openshift/library-go#1201

when that merges, we'll just change the version for openshift/library-go up above

adambkaplan · 2021-09-13T18:02:06Z

+				case "CSIDriverSharedResource":
+					foundCSIDriverSharedResource = true


We don't need to check this feature gate - this gate is used to install the shared resource CSI driver on the cluster.

updated @adambkaplan PTAL

adambkaplan · 2021-09-13T18:02:42Z

-			SecurityClient: securityClient.SecurityV1(),
+			Image:                    imageTemplate.ExpandOrDie("docker-builder"),
+			SecurityClient:           securityClient.SecurityV1(),
+			BuildCSIVolumesAvailable: foundBuildCSIVolumes && foundCSIDriverSharedResource,


Only check the build flag:

Suggested change

BuildCSIVolumesAvailable: foundBuildCSIVolumes && foundCSIDriverSharedResource,

BuildCSIVolumesAvailable: foundBuildCSIVolumes,

updated @adambkaplan PTAL

gabemontero · 2021-09-13T19:50:25Z

/test e2e-aws

gabemontero · 2021-09-22T23:18:52Z

unrelated fails

/retest

gabemontero · 2021-09-29T13:45:43Z

/assign @adambkaplan

OK this has been updated with the merged versions of openshift/api and openshift/library-go and is ready for approve and/or review

gabemontero · 2021-09-29T14:18:50Z

unit test appears to be unrelated flake that passes for me locally ... let's see

/test unit

gabemontero · 2021-09-29T17:28:27Z

/retest

gabemontero · 2021-09-29T21:12:04Z

/retest

gabemontero · 2021-09-30T13:13:12Z

/retest

gabemontero · 2021-09-30T14:40:00Z

install time networking fail in e2e-aws-proxy

will wait on retest in case our aws is overloaded

gabemontero · 2021-09-30T17:39:05Z

inability to access registry.redhat.io with latest e2e-gcp-builds

/retest

gabemontero · 2021-09-30T21:00:22Z

less than before, but still inability to access registry.redhat.io with latest e2e-gcp-builds

/retest

adambkaplan

/approve

Generally looks good. Main suggestion is to use the phrase "CSIVolumesEnabled" instead of "Available" - I think this is better communicates intent.

adambkaplan · 2021-10-01T16:12:00Z

 	buildDefaults            builddefaults.BuildDefaults
 	buildOverrides           buildoverrides.BuildOverrides
 	internalRegistryHostname string
+	buildCSIVolumesAvailable bool


Suggested change

buildCSIVolumesAvailable bool

buildCSIVolumesEnabled bool

adambkaplan · 2021-10-01T16:12:13Z

 type DockerBuildStrategy struct {
-	Image string
+	Image                    string
+	BuildCSIVolumesAvailable bool


Suggested change

BuildCSIVolumesAvailable bool

BuildCSIVolumesEnabled bool

adambkaplan · 2021-10-01T16:12:35Z

 	// setupContainersNodeStorage(pod, &pod.Spec.Containers[0]) // for privileged builds
 	setupBlobCache(pod)
-	if err := setupBuildVolumes(pod, build.Spec.Strategy.DockerStrategy.Volumes); err != nil {
+	if err := setupBuildVolumes(pod, build.Spec.Strategy.DockerStrategy.Volumes, bs.BuildCSIVolumesAvailable); err != nil {


Suggested change

if err := setupBuildVolumes(pod, build.Spec.Strategy.DockerStrategy.Volumes, bs.BuildCSIVolumesAvailable); err != nil {

if err := setupBuildVolumes(pod, build.Spec.Strategy.DockerStrategy.Volumes, bs.BuildCSIVolumesEnabled); err != nil {

adambkaplan · 2021-10-01T16:12:51Z

-	SecurityClient securityclient.SecurityV1Interface
+	Image                    string
+	SecurityClient           securityclient.SecurityV1Interface
+	BuildCSIVolumesAvailable bool


Suggested change

BuildCSIVolumesAvailable bool

BuildCSIVolumesEnabled bool

adambkaplan · 2021-10-01T16:13:05Z

 	// setupContainersNodeStorage(pod, &pod.Spec.Containers[0]) // for privileged builds
 	setupBlobCache(pod)
-	if err := setupBuildVolumes(pod, build.Spec.Strategy.SourceStrategy.Volumes); err != nil {
+	if err := setupBuildVolumes(pod, build.Spec.Strategy.SourceStrategy.Volumes, bs.BuildCSIVolumesAvailable); err != nil {


Suggested change

if err := setupBuildVolumes(pod, build.Spec.Strategy.SourceStrategy.Volumes, bs.BuildCSIVolumesAvailable); err != nil {

if err := setupBuildVolumes(pod, build.Spec.Strategy.SourceStrategy.Volumes, bs.BuildCSIVolumesEnabled); err != nil {

adambkaplan · 2021-10-01T16:13:28Z


 // setupBuildVolumes sets up user defined BuildVolumes
-func setupBuildVolumes(pod *corev1.Pod, buildVolumes []buildv1.BuildVolume) error {
+func setupBuildVolumes(pod *corev1.Pod, buildVolumes []buildv1.BuildVolume, buildCSIVolumes bool) error {


Suggested change

func setupBuildVolumes(pod *corev1.Pod, buildVolumes []buildv1.BuildVolume, buildCSIVolumes bool) error {

func setupBuildVolumes(pod *corev1.Pod, buildVolumes []buildv1.BuildVolume, csiVolumesEnabled bool) error {

adambkaplan · 2021-10-01T16:14:21Z

+	foundBuildCSIVolumes := false
+	if fg != nil {
+		for _, v := range fg {
+			v = strings.TrimSpace(v)
+			if v == "BuildCSIVolumes=true" {
+				foundBuildCSIVolumes = true
+			}
+		}
+	}


Suggested change

foundBuildCSIVolumes := false

if fg != nil {

for _, v := range fg {

v = strings.TrimSpace(v)

if v == "BuildCSIVolumes=true" {

foundBuildCSIVolumes = true

}

}

}

csiVolumesEnabled := false

if fg != nil {

for _, v := range fg {

v = strings.TrimSpace(v)

if v == "BuildCSIVolumes=true" {

csiVolumesEnabled = true

}

}

}

adambkaplan · 2021-10-01T16:14:48Z

 		DockerBuildStrategy: &buildstrategy.DockerBuildStrategy{
-			Image: imageTemplate.ExpandOrDie("docker-builder"),
+			Image:                    imageTemplate.ExpandOrDie("docker-builder"),
+			BuildCSIVolumesAvailable: foundBuildCSIVolumes,


Suggested change

BuildCSIVolumesAvailable: foundBuildCSIVolumes,

BuildCSIVolumesEnabled: csiVolumesEnabled,

adambkaplan · 2021-10-01T16:15:10Z

-			SecurityClient: securityClient.SecurityV1(),
+			Image:                    imageTemplate.ExpandOrDie("docker-builder"),
+			SecurityClient:           securityClient.SecurityV1(),
+			BuildCSIVolumesAvailable: foundBuildCSIVolumes,


Suggested change

BuildCSIVolumesAvailable: foundBuildCSIVolumes,

BuildCSIVolumesEnabled: csiVolumesEnabled,

…olumes

gabemontero · 2021-10-01T17:05:09Z

renames pulled and re pushed, commits squashed @adambkaplan

ptal / thanks

adambkaplan

/lgtm

openshift-ci · 2021-10-01T20:35:42Z

[APPROVALNOTIFIER] This PR is APPROVED

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

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

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

openshift-bot · 2021-10-01T20:57:44Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-10-01T21:21:45Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

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 Sep 10, 2021

openshift-ci Bot requested review from adambkaplan and smarterclayton September 10, 2021 17:34

gabemontero commented Sep 10, 2021

View reviewed changes

gabemontero mentioned this pull request Sep 10, 2021

BUILD-281: add feature gate, OCM configuration bit, to enable build csi volume tech preview openshift/api#1007

Merged

adambkaplan reviewed Sep 13, 2021

View reviewed changes

gabemontero force-pushed the bld-csi-vol-mnt-cfg-ocm branch from 3ff27d1 to ea0f01d Compare September 13, 2021 18:30

openshift-ci Bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 22, 2021

gabemontero force-pushed the bld-csi-vol-mnt-cfg-ocm branch from d1c095f to 932116a Compare September 22, 2021 19:05

openshift-ci Bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 22, 2021

bump(*): openshift/api,library-go

83af2f3

gabemontero force-pushed the bld-csi-vol-mnt-cfg-ocm branch 2 times, most recently from fc5cce3 to fdcc9d6 Compare September 29, 2021 13:44

gabemontero changed the title ~~WIP: BUILD-281: wire in OCM feature gate config fields to allow eventual use of CSI volumes~~ BUILD-281: wire in OCM feature gate config fields to allow eventual use of CSI volumes when BUILD-275 lands Sep 29, 2021

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 Sep 29, 2021

openshift-ci Bot assigned adambkaplan Sep 29, 2021

adambkaplan reviewed Oct 1, 2021

View reviewed changes

openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 1, 2021

wire in OCM feature gate config fields to allow eventual use of CSI v…

ebe9867

…olumes

gabemontero force-pushed the bld-csi-vol-mnt-cfg-ocm branch from fdcc9d6 to ebe9867 Compare October 1, 2021 17:04

adambkaplan approved these changes Oct 1, 2021

View reviewed changes

openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Oct 1, 2021

openshift-merge-robot merged commit fd48f0a into openshift:master Oct 1, 2021

gabemontero deleted the bld-csi-vol-mnt-cfg-ocm branch October 2, 2021 01:02


		replace github.com/openshift/api => github.com/gabemontero/api v0.0.0-20210910154316-eaa29e1f16df

		replace github.com/openshift/library-go => github.com/gabemontero/library-go v0.0.0-20210910154638-8565ba189275

		case "CSIDriverSharedResource":
		foundCSIDriverSharedResource = true

	BuildCSIVolumesAvailable: foundBuildCSIVolumes && foundCSIDriverSharedResource,
	BuildCSIVolumesAvailable: foundBuildCSIVolumes,

	if err := setupBuildVolumes(pod, build.Spec.Strategy.DockerStrategy.Volumes, bs.BuildCSIVolumesAvailable); err != nil {
	if err := setupBuildVolumes(pod, build.Spec.Strategy.DockerStrategy.Volumes, bs.BuildCSIVolumesEnabled); err != nil {

	func setupBuildVolumes(pod *corev1.Pod, buildVolumes []buildv1.BuildVolume, buildCSIVolumes bool) error {
	func setupBuildVolumes(pod *corev1.Pod, buildVolumes []buildv1.BuildVolume, csiVolumesEnabled bool) error {

	BuildCSIVolumesAvailable: foundBuildCSIVolumes,
	BuildCSIVolumesEnabled: csiVolumesEnabled,

Conversation

gabemontero commented Sep 10, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gabemontero commented Sep 13, 2021

Uh oh!

gabemontero commented Sep 22, 2021

Uh oh!

gabemontero commented Sep 29, 2021

Uh oh!

gabemontero commented Sep 29, 2021

Uh oh!

gabemontero commented Sep 29, 2021

Uh oh!

gabemontero commented Sep 29, 2021

Uh oh!

gabemontero commented Sep 30, 2021

Uh oh!

gabemontero commented Sep 30, 2021

Uh oh!

gabemontero commented Sep 30, 2021

Uh oh!

gabemontero commented Sep 30, 2021

Uh oh!

adambkaplan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gabemontero commented Oct 1, 2021

Uh oh!

adambkaplan left a comment

Choose a reason for hiding this comment

Uh oh!

openshift-ci Bot commented Oct 1, 2021

Uh oh!

openshift-bot commented Oct 1, 2021

Uh oh!

openshift-bot commented Oct 1, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants