Added support for Kaniko external registries #1495

FelGel · 2020-02-09T17:43:47Z

No description provided.

liranbg · 2020-02-10T11:38:34Z

#1448

omesser · 2020-02-10T12:42:12Z

@FelGel - We're releasing soon so no need to bump helm chart version (we're bumping on release)

…edentials

omesser · 2020-02-12T14:44:46Z

hack/k8s/helm/nuclio/templates/deployment/dashboard.yaml

+          value: {{ .Values.registry.defaultBaseRegistryURL }}
+        {{- end }}
+        {{- if .Values.registry.cacheRepo }}
+        - name: NUCLIO_DASHBOARD_CACHE_REPO


Suggested change

- name: NUCLIO_DASHBOARD_CACHE_REPO

- name: NUCLIO_DASHBOARD_KANIKO_CACHE_REPO

omesser · 2020-02-12T14:44:57Z

pkg/platform/factory/factory.go

 	}

+	containerBuilderConfiguration.CacheRepo = common.GetEnvOrDefaultString("NUCLIO_DASHBOARD_CACHE_REPO", "")


Suggested change

containerBuilderConfiguration.CacheRepo = common.GetEnvOrDefaultString("NUCLIO_DASHBOARD_CACHE_REPO", "")

containerBuilderConfiguration.CacheRepo = common.GetEnvOrDefaultString("NUCLIO_DASHBOARD_KANIKO_CACHE_REPO", "")

omesser · 2020-02-12T14:49:19Z

hack/k8s/helm/nuclio/templates/deployment/dashboard.yaml

@@ -80,6 +92,13 @@ spec:
        - name: NUCLIO_TEMPLATES_GIT_REF
          value: "nil"
        {{- end }}
+        {{- if .Values.registry.secretName }}
+        - name: NUCLIO_REGISTRY_CREDENTIALS_SECRET_NAME
+          value: {{ .Values.registry.secretName }}


-> hide {{ .Values.registry.secretName }} behind nuclio.registryCredentialsName in _helpers.tpl

omesser · 2020-02-12T14:58:52Z

pkg/containerimagebuilderpusher/kaniko.go

@@ -171,17 +175,23 @@ func (k *Kaniko) getKanikoJobSpec(namespace string, buildOptions *BuildOptions,
 		buildArgs = append(buildArgs, "--cache=true")


Disable caching by default until we understand why it doesn't use it as we expected

That is configurable correct here. disabling ideally should happen via UI

@liranbg is write, this flag is configurable
won't force disable

*right :)

As discussed, if this is not broken as previously tested, no objection to hook it up to the cache option in function config

omesser · 2020-02-12T15:09:28Z

pkg/containerimagebuilderpusher/kaniko.go

+			Name: "docker-config",
+			VolumeSource: v1.VolumeSource{
+				Secret: &v1.SecretVolumeSource{
+					SecretName: k.builderConfiguration.RegistryCredentialsSecretName,


Take from function spec

omesser · 2020-02-12T15:10:16Z

pkg/platform/abstract/platform.go

@@ -158,6 +161,9 @@ func (ap *Platform) HandleDeployFunction(existingFunctionConfig *functionconfig.
 		if err = onAfterConfigUpdatedWrapper(&createFunctionOptions.FunctionConfig); err != nil {
 			return nil, errors.Wrap(err, "Failed to trigger on after config update")
 		}
+
+		// Update function config with ImagePull secrets so image can be pulled from docker registry
+		createFunctionOptions.FunctionConfig.Spec.ImagePullSecrets = ap.platform.GetSecretName()


If you enrich before build, this is redundant

omesser · 2020-02-12T15:11:29Z

pkg/processor/build/builder.go

@@ -246,6 +246,8 @@ func (b *Builder) Build(options *platform.CreateFunctionBuildOptions) (*platform
 		enrichedConfiguration.Spec.Image = processorImage
 	}

+	enrichedConfiguration.Spec.ImagePullSecrets = b.platform.GetSecretName()


can be removed

omesser · 2020-02-12T15:21:04Z

pkg/platform/abstract/platform.go

@@ -132,6 +132,9 @@ func (ap *Platform) HandleDeployFunction(existingFunctionConfig *functionconfig.
 			// use the function configuration augmented by the builder
 			createFunctionOptions.FunctionConfig.Spec.Image = buildResult.Image

+			// Update function config with ImagePull secrets so image we just build can be pulled from docker registry
+			createFunctionOptions.FunctionConfig.Spec.ImagePullSecrets = buildResult.UpdatedFunctionConfig.Spec.ImagePullSecrets


condition if not exists (so you don't override) and move to not depend on functionBuildRequired
Also it's pushpull secret (pull for controller, push for kaniko/builder)
And since it's for the kaniko build - need to be enriched before line 121

liranbg

🚀
minor comments and question to @omesser below

liranbg · 2020-02-12T15:32:04Z

pkg/containerimagebuilderpusher/containerimagebuilder.go

@@ -16,4 +16,7 @@ type BuilderPusher interface {

 	// GetBaseImageRegistry returns onbuild base registry
 	GetBaseImageRegistry(registry string) string
+
+	// GetSecretName returns secret with credentials to push/pull from docker registry
+	GetSecretName() string


Suggested change

GetSecretName() string

GetRegistryCredentialsSecretName() string

I mean, secret is the abstraction name, not the instance :)

changed to GetDefaultRegistryCredentialsSecretName

liranbg · 2020-02-12T15:35:55Z

pkg/containerimagebuilderpusher/kaniko.go

@@ -171,17 +175,23 @@ func (k *Kaniko) getKanikoJobSpec(namespace string, buildOptions *BuildOptions,
 		buildArgs = append(buildArgs, "--cache=true")


That is configurable correct here. disabling ideally should happen via UI

liranbg · 2020-02-12T15:36:56Z

pkg/containerimagebuilderpusher/kaniko.go

@@ -275,7 +310,13 @@ func (k *Kaniko) waitForKanikoJobCompletion(namespace string, jobName string, Bu
 		}

 		if runningJob.Status.Succeeded > 0 {
-			k.logger.Debug("Kaniko job was completed successfully")
+			jobLogs, err := k.getJobLogs(namespace, jobName)


liranbg · 2020-02-12T15:42:17Z

pkg/containerimagebuilderpusher/kaniko.go

+				return nil
+			}
+
+			k.logger.Debug("Kaniko job was completed successfully", "logs", jobLogs)


Suggested change

k.logger.Debug("Kaniko job was completed successfully", "logs", jobLogs)

k.logger.Debug("Kaniko job was completed successfully", "jobLogs", jobLogs)

liranbg · 2020-02-12T15:45:35Z

pkg/platform/factory/factory.go

 	}
 	if containerBuilderConfiguration.BusyBoxImage == "" {
-		containerBuilderConfiguration.BusyBoxImage = common.GetEnvOrDefaultString("NUCLIO_BUSYBOX_CONTAINER_IMAGE", "busybox:1.31.0")
+		containerBuilderConfiguration.BusyBoxImage = common.GetEnvOrDefaultString("NUCLIO_BUSYBOX_CONTAINER_IMAGE",
+			"busybox:1.31.1")


Suggested change

"busybox:1.31.1")

"busybox:1.31")

Felix added 4 commits February 6, 2020 20:47

Kaniko credentials support

5c8f622

Bugfixing

2d8d8c6

Formatting

448c681

CR fixes

ae4e9d8

omesser self-requested a review February 10, 2020 12:42

omesser approved these changes Feb 10, 2020

View reviewed changes

Felix and others added 5 commits February 10, 2020 16:21

Reverting chart version bump

06c89a9

Lint fix

2697348

Propagated docker registry secret to function deploy

738a2ad

Will use default secret name if not defined

ff39c19

Added comment about copying secrets between namespaces

948b887

omesser added area/build area/controller labels Feb 11, 2020

FelGel added 3 commits February 12, 2020 12:06

Merge branch 'development' of github.com:nuclio/nuclio into kaniko_cr…

b27f82d

…edentials

Test fix + improved logging

93fd882

Added separate flags for insecure push and insecure pull registries

a35b468

omesser suggested changes Feb 12, 2020

View reviewed changes

liranbg reviewed Feb 12, 2020

View reviewed changes

Additional CR fixes

381ce39

omesser self-requested a review February 12, 2020 19:32

omesser approved these changes Feb 12, 2020

View reviewed changes

omesser merged commit d7a3d2d into nuclio:development Feb 12, 2020

liranbg mentioned this pull request Feb 13, 2020

Fix missing secretName value #1505

Merged

FelGel deleted the kaniko_credentials branch April 12, 2020 14:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added support for Kaniko external registries #1495

Added support for Kaniko external registries #1495

FelGel commented Feb 9, 2020

liranbg commented Feb 10, 2020

omesser commented Feb 10, 2020

omesser Feb 12, 2020

omesser Feb 12, 2020

omesser Feb 12, 2020

omesser Feb 12, 2020

liranbg Feb 12, 2020

FelGel Feb 12, 2020

omesser Feb 12, 2020

omesser Feb 12, 2020

omesser Feb 12, 2020

omesser Feb 12, 2020

omesser Feb 12, 2020

liranbg left a comment

liranbg Feb 12, 2020

FelGel Feb 12, 2020

liranbg Feb 12, 2020

liranbg Feb 12, 2020

liranbg Feb 12, 2020

liranbg Feb 12, 2020

	- name: NUCLIO_DASHBOARD_CACHE_REPO
	- name: NUCLIO_DASHBOARD_KANIKO_CACHE_REPO

		}

		containerBuilderConfiguration.CacheRepo = common.GetEnvOrDefaultString("NUCLIO_DASHBOARD_CACHE_REPO", "")

		@@ -171,17 +175,23 @@ func (k Kaniko) getKanikoJobSpec(namespace string, buildOptions BuildOptions,
		buildArgs = append(buildArgs, "--cache=true")

	GetSecretName() string
	GetRegistryCredentialsSecretName() string

	k.logger.Debug("Kaniko job was completed successfully", "logs", jobLogs)
	k.logger.Debug("Kaniko job was completed successfully", "jobLogs", jobLogs)

Added support for Kaniko external registries #1495

Added support for Kaniko external registries #1495

Conversation

FelGel commented Feb 9, 2020

liranbg commented Feb 10, 2020

omesser commented Feb 10, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

liranbg left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment