feat(openshift): container Build action #4726

Walther · 2023-06-28T14:57:48Z

What this PR does / why we need it:

Enables the use of a container Build action on OpenShift, with both local-docker and in-cluster kaniko builders.

Which issue(s) this PR fixes:

Fixes #4625

Special notes for your reviewer:

Instructions contained in the readme.

Walther · 2023-06-28T19:38:16Z

core/src/plugins/kubernetes/container/build/common.ts

-function isLocalHostname(hostname: string) {
-  return hostname === "localhost" || hostname.startsWith("127.")
-}
-


This function name sounds way more generic than its actual use was, and it was used only in one place. Inlined to getManifestInspectArgs (with a descriptive comment) to avoid accidental wrong use in other places

Walther · 2023-06-28T19:39:48Z

core/test/data/openshift/demo-project/garden.yml

+      # FIXME: this config option is overloaded
+      # in different places, it is used for both force HTTP as well as skip TLS verify on HTTPS
+      insecure: true


See core/src/plugins/kubernetes/container/build/kaniko.ts

Walther · 2023-06-28T19:41:30Z

core/test/data/openshift/demo-project/README.md

+_TODO: we should fix this properly by editing the `garden-util` image, and make this section obsolete_
+
+```bash
+oc login -u kubeadmin https://api.crc.testing:6443
+oc adm policy add-scc-to-user anyuid -z default --namespace demo-project
+oc logout
+oc login -u developer -p developer
+```


Task for later: make the garden-util pod run happily in a more security-constrained context, and remove the need for this workaround / additional permissions.

Walther · 2023-06-28T19:43:53Z

core/test/data/openshift/demo-project/hello/garden.yml

+# FIXME: currently this is required, otherwise you get an ENOENT when calling
+# containerHelpers.dockerCli({cwd: action.getBuildPath() ... })
+# because the .garden/build directory is not created
+buildAtSource: true


When trying to do local-docker builds but having the deploymentRegistry set, various methods in our codebase threw ENOENT due to spawn attempting to use a cwd of .garden/build/buildname but nothing had created it in advance. Took a quick look into it but didn't find anything obvious, decided to move on with a workaround to prioritize getting something done.

Walther · 2023-06-28T19:45:47Z

core/src/plugins/kubernetes/container/build/common.ts

+      // TODO: if a registry does not have an image with the name at all, we throw here
+      // This isn't a great first-time-use experience (or after you've reset a registry)


A task for later. This isn't critical, as the process actually continues afterwards, happily building and pushing the first image, but it isn't the best first-use experience to see a large red error on your first build.

Walther · 2023-06-28T19:47:37Z

core/src/plugins/kubernetes/container/build/common.ts

+  // Allow insecure connections on local registry
+  if (
+    hostname === "localhost" ||
+    hostname.startsWith("127.") ||
+    hostname === "default-route-openshift-image-registry.apps-crc.testing"
+  ) {


Fun fact: here insecure means e.g. allowing self-signed certs etc

Walther · 2023-06-28T19:48:30Z

core/src/plugins/kubernetes/container/build/kaniko.ts

+  }
+
+  // TODO: do we support the garden-provided in-cluster registry anymore, or could this be deleted?
+  if (provider.config.deploymentRegistry?.insecure === true && !isOpenShiftLocal) {


Fun fact: here insecure is used to force kaniko to use HTTP instead of HTTPS.

But OpenShift requires HTTPS (the registry is only served over it, you'll just get 503 service unavailable on HTTP).

We should probably make the config flag a bit clearer.

It looks like the in-cluster registry was removed in 67047e68.

Another potential case here is if people are using kaniko build and some self-deployed registry over HTTP - but not sure if that's too far-fetched.

Maybe we can leave this code as it is to avoid breaking changes, and potentially clean up later?

Yes, I think it's better to leave this code.

vvagaytsev · 2023-06-30T07:48:24Z

core/src/plugins/openshift/build.ts

+import { getLocalBuildStatus, localBuild } from "../kubernetes/container/build/local"
+
+export const openshiftContainerBuildExtension = (): BuildActionExtension<ContainerBuildAction> => ({
+  name: "container",


Just a question. Does name here always equal the value of the corresponding action type?

This is directly based on

garden/core/src/plugins/kubernetes/container/extensions.ts

Lines 40 to 42 in 47c8584

export const k8sContainerBuildExtension = (): BuildActionExtension<ContainerBuildAction> => ({

name: "container",

handlers: {

vvagaytsev

LGTM, thanks for the great job! 👍

* chore(openshift): rename container.ts -> deploy.ts * feat(openshift): container Build action, buildmode local-docker * chore: fix lint on openshift/build.ts * feat(openshift): in-cluster container Build using kaniko * chore(openshift): add openshift logo to example web page * chore(openshift): update demo-project readme

Walther added 4 commits June 26, 2023 14:33

chore(openshift): rename container.ts -> deploy.ts

3aae47e

feat(openshift): container Build action, buildmode local-docker

1e6c7a8

chore: fix lint on openshift/build.ts

d1ecd4a

feat(openshift): in-cluster container Build using kaniko

891cd6d

Walther commented Jun 28, 2023

View reviewed changes

Walther marked this pull request as ready for review June 28, 2023 19:53

Walther requested a review from a team June 28, 2023 19:53

Walther added 3 commits June 29, 2023 11:45

chore(openshift): add openshift logo to example web page

3099e45

chore(openshift): update demo-project readme

65241b3

Merge branch 'main' into local-openshift-build

47c8584

vvagaytsev reviewed Jun 30, 2023

View reviewed changes

vvagaytsev approved these changes Jun 30, 2023

View reviewed changes

Walther merged commit bdf9e0f into main Jun 30, 2023

Walther deleted the local-openshift-build branch June 30, 2023 09:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(openshift): container Build action #4726

feat(openshift): container Build action #4726

Walther commented Jun 28, 2023 •

edited

Loading

Walther Jun 28, 2023 •

edited

Loading

Walther Jun 28, 2023

Walther Jun 28, 2023

Walther Jun 28, 2023

Walther Jun 28, 2023 •

edited

Loading

Walther Jun 28, 2023

Walther Jun 28, 2023 •

edited

Loading

vvagaytsev Jun 30, 2023

Walther Jun 30, 2023

vvagaytsev Jun 30, 2023

vvagaytsev Jun 30, 2023

Walther Jun 30, 2023 •

edited

Loading

vvagaytsev left a comment

		// TODO: if a registry does not have an image with the name at all, we throw here
		// This isn't a great first-time-use experience (or after you've reset a registry)

	export const k8sContainerBuildExtension = (): BuildActionExtension<ContainerBuildAction> => ({
	name: "container",
	handlers: {

feat(openshift): container Build action #4726

feat(openshift): container Build action #4726

Conversation

Walther commented Jun 28, 2023 • edited Loading

Walther Jun 28, 2023 • edited Loading

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

Walther Jun 28, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Walther Jun 28, 2023 • edited Loading

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

Walther Jun 30, 2023 • edited Loading

Choose a reason for hiding this comment

vvagaytsev left a comment

Choose a reason for hiding this comment

Walther commented Jun 28, 2023 •

edited

Loading

Walther Jun 28, 2023 •

edited

Loading

Walther Jun 28, 2023 •

edited

Loading

Walther Jun 28, 2023 •

edited

Loading

Walther Jun 30, 2023 •

edited

Loading