Skip to content

🌱 Test Registry #1425

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 12 additions & 14 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -165,12 +165,16 @@ test-unit: $(SETUP_ENVTEST) #HELP Run the unit tests
$(UNIT_TEST_DIRS) \
-test.gocoverdir=$(ROOT_DIR)/coverage/unit

image-registry: ## Setup in-cluster image registry
./hack/test/image-registry.sh $(E2E_REGISTRY_NAMESPACE) $(E2E_REGISTRY_NAME)

build-push-e2e-catalog: ## Build the testdata catalog used for e2e tests and push it to the image registry
./hack/test/build-push-e2e-catalog.sh $(E2E_REGISTRY_NAMESPACE) $(LOCAL_REGISTRY_HOST)/$(E2E_TEST_CATALOG_V1)
./hack/test/build-push-e2e-catalog.sh $(E2E_REGISTRY_NAMESPACE) $(LOCAL_REGISTRY_HOST)/$(E2E_TEST_CATALOG_V2)
.PHONY: image-registry
E2E_REGISTRY_IMAGE=localhost/e2e-test-registry:devel
image-registry: export GOOS=linux
image-registry: export GOARCH=amd64
image-registry: ## Build the testdata catalog used for e2e tests and push it to the image registry
go build $(GO_BUILD_FLAGS) -tags '$(GO_BUILD_TAGS)' -ldflags '$(GO_BUILD_LDFLAGS)' -gcflags '$(GO_BUILD_GCFLAGS)' -asmflags '$(GO_BUILD_ASMFLAGS)' -o ./testdata/registry/bin/registry ./testdata/registry/registry.go
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
go build $(GO_BUILD_FLAGS) -tags '$(GO_BUILD_TAGS)' -ldflags '$(GO_BUILD_LDFLAGS)' -gcflags '$(GO_BUILD_GCFLAGS)' -asmflags '$(GO_BUILD_ASMFLAGS)' -o ./testdata/registry/bin/registry ./testdata/registry/registry.go
GOOS=linux GOARCH=amd64 go build $(GO_BUILD_FLAGS) -tags '$(GO_BUILD_TAGS)' -ldflags '$(GO_BUILD_LDFLAGS)' -gcflags '$(GO_BUILD_GCFLAGS)' -asmflags '$(GO_BUILD_ASMFLAGS)' -o ./testdata/registry/bin/registry ./testdata/registry/registry.go

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the test-e2e target has a dependency on the image-registry target, the GOOS and GOARCH variables should be set from the test-e2e target, and don't need to be set explicitly here. This way, you can build image-registry locally, and get the correct (local) images when no running the e2e.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @tmshort and @bentito,

Would it make more sense to use GOOS=$(go env GOOS) GOARCH=$(go env GOARCH) instead?

This approach would make the target compatible with both macOS and Linux environments, allowing us to run it locally as well. If we explicitly set GOARCH=amd64, for example, it won’t run on macOS systems with a different architecture, where the expected result would be darwin/arm64. Is targeting linux/amd64 specifically what we're aiming for here?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep. current changes take this into account.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure that's the right approach. Doing the export should work when doing the image-target, and setting the variable equal to the go env ought to be a noop.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @tmshort,

Not sure that's the right approach. Doing the export should work when doing the image-target, and setting the variable equal to the go env ought to be a noop.

I’m not sure if I fully follow your comment here. The export has fixed values as well:

image-registry: export GOOS=linux
image-registry: export GOARCH=amd64

Either way, this suggests fixed values for go build.

So, how would GOOS=linux GOARCH=amd64 go build build a binary for darwin/arm64 (my local env, for example)?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, but doing this:

GOOS=$(go env GOOS)

Is basically setting the GOOS variable to the environment value that the go command already has available to it. It's equivalent to doing:

GOOS=${GOOS}

Which is a no-op.

Copy link
Contributor

@tmshort tmshort Nov 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@camilamacedo86 I didn't answer your question.

So, how would GOOS=linux GOARCH=amd64 go build build a binary for darwin/arm64 (my local env, for example)?

It wouldn't. In this particular case, it's building executables for the docker container, which have to be linux/amd64. These two executables are never expected to run locally.

My point is that doing GOOS=$(go env GODS) is pointless.

Copy link
Contributor Author

@dtfranz dtfranz Nov 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This build is specifically for the image-registry image to be run in kind; comparing to how we build the operator-controller's manager binary, we use the same GOOS=linux GOARCH=amd64 ENV when building (make run uses docker-build which then uses build-linux). I'm just following that same convention.

If you feel it's important, what we could potentially do is split up the image-registry target so that GOOS=linux GOARCH=amd64 is only set when actually building the image. But I think this is unnecessary or at least non-critical enough that we could address it later in a follow-up.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These two executables are never expected to run locally.

If that is the case, (we never run it locally and would not make sense run it locally) then I am OK with fixed values 👍

go build $(GO_BUILD_FLAGS) -tags '$(GO_BUILD_TAGS)' -ldflags '$(GO_BUILD_LDFLAGS)' -gcflags '$(GO_BUILD_GCFLAGS)' -asmflags '$(GO_BUILD_ASMFLAGS)' -o ./testdata/push/bin/push ./testdata/push/push.go
$(CONTAINER_RUNTIME) build -f ./testdata/Dockerfile -t $(E2E_REGISTRY_IMAGE) ./testdata
$(CONTAINER_RUNTIME) save $(E2E_REGISTRY_IMAGE) | $(KIND) load image-archive /dev/stdin --name $(KIND_CLUSTER_NAME)
./testdata/build-test-registry.sh $(E2E_REGISTRY_NAMESPACE) $(E2E_REGISTRY_NAME) $(E2E_REGISTRY_IMAGE)

# When running the e2e suite, you can set the ARTIFACT_PATH variable to the absolute path
# of the directory for the operator-controller e2e tests to store the artifacts, which
Expand All @@ -181,7 +185,7 @@ build-push-e2e-catalog: ## Build the testdata catalog used for e2e tests and pus
test-e2e: KIND_CLUSTER_NAME := operator-controller-e2e
test-e2e: KUSTOMIZE_BUILD_DIR := config/overlays/e2e
test-e2e: GO_BUILD_FLAGS := -cover
test-e2e: run image-registry build-push-e2e-catalog registry-load-bundles e2e e2e-coverage kind-clean #HELP Run e2e test suite on local kind cluster
test-e2e: run image-registry e2e e2e-coverage kind-clean #HELP Run e2e test suite on local kind cluster

.PHONY: extension-developer-e2e
extension-developer-e2e: KUSTOMIZE_BUILD_DIR := config/overlays/cert-manager
Expand All @@ -205,7 +209,7 @@ post-upgrade-checks:
test-upgrade-e2e: KIND_CLUSTER_NAME := operator-controller-upgrade-e2e
test-upgrade-e2e: export TEST_CLUSTER_CATALOG_NAME := test-catalog
test-upgrade-e2e: export TEST_CLUSTER_EXTENSION_NAME := test-package
test-upgrade-e2e: kind-cluster run-latest-release image-registry build-push-e2e-catalog registry-load-bundles pre-upgrade-setup docker-build kind-load kind-deploy post-upgrade-checks kind-clean #HELP Run upgrade e2e tests on a local kind cluster
test-upgrade-e2e: kind-cluster run-latest-release image-registry pre-upgrade-setup docker-build kind-load kind-deploy post-upgrade-checks kind-clean #HELP Run upgrade e2e tests on a local kind cluster

.PHONY: e2e-coverage
e2e-coverage:
Expand All @@ -231,12 +235,6 @@ kind-cluster: $(KIND) #EXHELP Standup a kind cluster.
kind-clean: $(KIND) #EXHELP Delete the kind cluster.
$(KIND) delete cluster --name $(KIND_CLUSTER_NAME)

registry-load-bundles: ## Load selected e2e testdata container images created in kind-load-bundles into registry
testdata/bundles/registry-v1/build-push-e2e-bundle.sh ${E2E_REGISTRY_NAMESPACE} $(LOCAL_REGISTRY_HOST)/bundles/registry-v1/prometheus-operator:v1.0.0 prometheus-operator.v1.0.0 prometheus-operator.v1.0.0
testdata/bundles/registry-v1/build-push-e2e-bundle.sh ${E2E_REGISTRY_NAMESPACE} $(LOCAL_REGISTRY_HOST)/bundles/registry-v1/prometheus-operator:v1.0.1 prometheus-operator.v1.0.1 prometheus-operator.v1.0.0
testdata/bundles/registry-v1/build-push-e2e-bundle.sh ${E2E_REGISTRY_NAMESPACE} $(LOCAL_REGISTRY_HOST)/bundles/registry-v1/prometheus-operator:v1.2.0 prometheus-operator.v1.2.0 prometheus-operator.v1.0.0
testdata/bundles/registry-v1/build-push-e2e-bundle.sh ${E2E_REGISTRY_NAMESPACE} $(LOCAL_REGISTRY_HOST)/bundles/registry-v1/prometheus-operator:v2.0.0 prometheus-operator.v2.0.0 prometheus-operator.v1.0.0

#SECTION Build

ifeq ($(origin VERSION), undefined)
Expand Down
66 changes: 0 additions & 66 deletions hack/test/build-push-e2e-catalog.sh

This file was deleted.

2 changes: 1 addition & 1 deletion test/e2e/cluster_extension_install_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -550,7 +550,7 @@ func TestClusterExtensionInstallReResolvesWhenCatalogIsPatched(t *testing.T) {

// patch imageRef tag on test-catalog image with v2 image
t.Log("By patching the catalog ImageRef to point to the v2 catalog")
updatedCatalogImage := fmt.Sprintf("%s/e2e/test-catalog:v2", os.Getenv("LOCAL_REGISTRY_HOST"))
updatedCatalogImage := fmt.Sprintf("%s/test-catalog:v2", os.Getenv("LOCAL_REGISTRY_HOST"))
err := patchTestCatalog(context.Background(), testCatalogName, updatedCatalogImage)
require.NoError(t, err)
require.EventuallyWithT(t, func(ct *assert.CollectT) {
Expand Down
2 changes: 2 additions & 0 deletions testdata/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
push/bin
registry/bin
12 changes: 12 additions & 0 deletions testdata/Dockerfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
from gcr.io/distroless/static:nonroot

WORKDIR /

COPY registry/bin/registry registry
COPY push/bin/push push

COPY images images

EXPOSE 5000

USER 65532:65532
51 changes: 44 additions & 7 deletions hack/test/image-registry.sh → testdata/build-test-registry.sh
Original file line number Diff line number Diff line change
@@ -1,29 +1,29 @@
#! /bin/bash
#!/bin/bash

set -o errexit
set -o nounset
set -o pipefail

help="
image-registry.sh is a script to stand up an image registry within a cluster.
build-test-registry.sh is a script to stand up an image registry within a cluster.
Usage:
image-registry.sh [NAMESPACE] [NAME] [CERT_REF]
build-test-registry.sh [NAMESPACE] [NAME] [IMAGE]

Argument Descriptions:
- NAMESPACE is the namespace that should be created and is the namespace in which the image registry will be created
- NAME is the name that should be used for the image registry Deployment and Service
- CERT_REF is the reference to the CA certificate that should be used to serve the image registry over HTTPS, in the
format of 'Issuer/<issuer-name>' or 'ClusterIssuer/<cluster-issuer-name>'
- IMAGE is the name of the image that should be used to run the image registry
"

if [[ "$#" -ne 2 ]]; then
if [[ "$#" -ne 3 ]]; then
echo "Illegal number of arguments passed"
echo "${help}"
exit 1
fi

namespace=$1
name=$2
image=$3

kubectl apply -f - << EOF
apiVersion: v1
Expand Down Expand Up @@ -69,7 +69,12 @@ spec:
spec:
containers:
- name: registry
image: registry:2
image: ${image}
imagePullPolicy: IfNotPresent
command:
- /registry
args:
- "--registry-address=:5000"
volumeMounts:
- name: certs-vol
mountPath: "/certs"
Expand Down Expand Up @@ -100,3 +105,35 @@ spec:
EOF

kubectl wait --for=condition=Available -n "${namespace}" "deploy/${name}" --timeout=60s

kubectl apply -f - << EOF
apiVersion: batch/v1
kind: Job
metadata:
name: ${name}-push
namespace: "${namespace}"
spec:
template:
spec:
restartPolicy: Never
containers:
- name: push
image: ${image}
command:
- /push
args:
- "--registry-address=${name}.${namespace}.svc:5000"
- "--images-path=/images"
volumeMounts:
- name: certs-vol
mountPath: "/certs"
env:
- name: SSL_CERT_DIR
value: "/certs/"
volumes:
- name: certs-vol
secret:
secretName: ${namespace}-registry
EOF

kubectl wait --for=condition=Complete -n "${namespace}" "job/${name}-push" --timeout=60s
84 changes: 0 additions & 84 deletions testdata/bundles/registry-v1/build-push-e2e-bundle.sh

This file was deleted.

This file was deleted.

6 changes: 0 additions & 6 deletions testdata/catalogs/test-catalog-v1.Dockerfile

This file was deleted.

6 changes: 0 additions & 6 deletions testdata/catalogs/test-catalog-v2.Dockerfile

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ properties:
schema: olm.bundle
name: prometheus-operator.1.0.1
package: prometheus
image: docker-registry.operator-controller-e2e.svc.cluster.local:5000/bundles/registry-v1/prometheus-operator:v1.0.1
image: docker-registry.operator-controller-e2e.svc.cluster.local:5000/bundles/registry-v1/prometheus-operator:v1.0.0
properties:
- type: olm.package
value:
Expand All @@ -42,7 +42,7 @@ properties:
schema: olm.bundle
name: prometheus-operator.1.2.0
package: prometheus
image: docker-registry.operator-controller-e2e.svc.cluster.local:5000/bundles/registry-v1/prometheus-operator:v1.2.0
image: docker-registry.operator-controller-e2e.svc.cluster.local:5000/bundles/registry-v1/prometheus-operator:v1.0.0
properties:
- type: olm.package
value:
Expand All @@ -63,7 +63,7 @@ entries:
schema: olm.bundle
name: prometheus-mirrored-operator.1.2.0
package: prometheus-mirrored
image: mirrored-registry.operator-controller-e2e.svc.cluster.local:5000/bundles/registry-v1/prometheus-operator:v1.2.0
image: mirrored-registry.operator-controller-e2e.svc.cluster.local:5000/bundles/registry-v1/prometheus-operator:v1.0.0
properties:
- type: olm.package
value:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ entries:
schema: olm.bundle
name: prometheus-operator.2.0.0
package: prometheus
image: docker-registry.operator-controller-e2e.svc.cluster.local:5000/bundles/registry-v1/prometheus-operator:v2.0.0
image: docker-registry.operator-controller-e2e.svc.cluster.local:5000/bundles/registry-v1/prometheus-operator:v1.0.0
properties:
- type: olm.package
value:
Expand Down
Loading
Loading