remove builder image dependencies on origin by bparees · Pull Request #20885 · openshift/origin

bparees · 2018-09-06T22:15:12Z

remaining imports:

$ grep -ri origin/pkg | grep -v origin/pkg/build/builder
<none>

$ grep -ri kubernetes/pkg | grep -v origin/pkg/build/builder
dockerutil.go:	"k8s.io/kubernetes/pkg/credentialprovider"
dockerutil.go:	"k8s.io/kubernetes/pkg/util/interrupt"
cmd/dockercfg/cfg.go:	"k8s.io/kubernetes/pkg/credentialprovider"

bparees · 2018-09-06T22:15:50Z

/assign @smarterclayton

bparees · 2018-09-07T03:21:24Z

/retest

mfojtik · 2018-09-07T11:44:26Z

/retest

mfojtik · 2018-09-07T13:19:50Z

/retest

(centos mirrors)

bparees · 2018-09-07T20:05:57Z

/retest

bparees · 2018-09-07T22:14:19Z

/retest

bparees · 2018-09-08T03:33:18Z

/retest

smarterclayton · 2018-09-10T20:07:18Z

pkg/build/builder/cmd/builder.go

-	legacy.InstallInternalLegacyBuild(buildEnvVarScheme)
-	utilruntime.Must(buildapiv1.AddToScheme(buildEnvVarScheme))
-	buildEnvVarJSONCodec = buildEnvVarCodecFactory.LegacyCodec(buildapiv1.SchemeGroupVersion, legacy.GroupVersion)
+	utilruntime.Must(buildapiv1.AddToScheme(buildScheme))


Wait, why not just use the scheme in the build client? github.com/openshift/client-go/build/clientset/versioned/scheme

Then you don't need this or to define your own scheme.

smarterclayton · 2018-09-10T20:07:58Z

pkg/build/builder/cmd/scmauth/util.go

 	}

-	lines, err := file.ReadLines(gitconfig)
+	lines, err := builder.ReadLines(gitconfig)


This method also seems like a library-go candidate

will do, but the fact that i was able to move it would imply no one else is using it today.

@deads2k veto'd putting this in library-go so i'm leaving it here.

This method also seems like a library-go candidate

@deads2k veto'd putting this in library-go so i'm leaving it here.

This is the only consumer and the method is questionable complexity for library-go.

smarterclayton · 2018-09-10T20:08:06Z

pkg/build/builder/origin-common/image/apis/image/reference/doc.go

@@ -0,0 +1,5 @@
+// digest is a copy from "github.com/docker/distribution/digest" that is kept because we want to avoid the godep,


I thought this was going to library-go?

bparees · 2018-09-11T02:03:56Z

/retest

bparees · 2018-09-12T00:06:01Z

/retest

bparees · 2018-09-12T18:01:04Z

glide.lock

  - pkg/util/proto/validation
 - name: k8s.io/kubernetes
-  version: 91becade863adc3e9a01585d027896fd1f0d12a7
+  version: c388e666caa7903dff9d4da6a4f850346d402da3


@deads2k remind me again why this shouldn't worry me?

it's because we sync levels back. As long as you don't see content change in k8s.io/kube, this is ok.

bparees · 2018-09-12T18:01:19Z

@openshift/sig-developer-experience ptal

smarterclayton · 2018-09-12T18:17:59Z

pkg/build/builder/common.go

 	containerNamePrefix = "openshift"
+	// ConfigMapBuildSourceBaseMountPath is the path that the controller will have
+	// mounted configmap input content within the build pod
+	ConfigMapBuildSourceBaseMountPath = "/var/run/configs/openshift.io/build"


Do these need to be public?

no, updated.

smarterclayton · 2018-09-12T18:18:50Z

pkg/build/builder/cmd/dockercfg/cfg.go

 	cfg, err = GetDockerConfig(searchPaths)
 	if err != nil {
-		glog.Errorf("Reading docker config from %v failed: %v", searchPaths, err)
+		glogreal.Errorf("Reading docker config from %v failed: %v", searchPaths, err)


Why is this glogreal?

because the glog wrapper doesn't implement "Errorf" and since the glog wrapper's would send anything at the "error" level to glog anyway, it was easier to just call real glog directly rather than add a pointless Errorf delegator to the glog wrapper.

i may have gotten that backwards.

in any case this was logging direct to glog before, and now it still will, but all the other glog logic is using the wrapper(where it previously was not)

and since glog wrapper is only intended to pretty print info messages, i think sending an error to "real" glog is the right thing to do.

openshift-bot · 2018-09-15T07:14:20Z

/retest