-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Scorecard custom image update and relocation #3711
Changes from 10 commits
6f8efaf
63bd71a
1fca3d2
54475ba
0b554fc
323c1c8
4faedcb
e4cf69a
e773509
921da4a
7fde48e
8064b44
4eb0115
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -29,18 +29,21 @@ GO_BUILD_ARGS = \ | |
|
||
ANSIBLE_BASE_IMAGE = quay.io/operator-framework/ansible-operator | ||
HELM_BASE_IMAGE = quay.io/operator-framework/helm-operator | ||
CUSTOM_SCORECARD_TESTS_BASE_IMAGE = quay.io/operator-framework/custom-scorecard-tests | ||
SCORECARD_TEST_BASE_IMAGE = quay.io/operator-framework/scorecard-test | ||
SCORECARD_TEST_KUTTL_BASE_IMAGE = quay.io/operator-framework/scorecard-test-kuttl | ||
|
||
ANSIBLE_IMAGE ?= $(ANSIBLE_BASE_IMAGE) | ||
HELM_IMAGE ?= $(HELM_BASE_IMAGE) | ||
CUSTOM_SCORECARD_TESTS_IMAGE ?= $(CUSTOM_SCORECARD_TESTS_BASE_IMAGE) | ||
SCORECARD_TEST_IMAGE ?= $(SCORECARD_TEST_BASE_IMAGE) | ||
SCORECARD_TEST_KUTTL_IMAGE ?= $(SCORECARD_TEST_KUTTL_BASE_IMAGE) | ||
|
||
ANSIBLE_ARCHES:="amd64" "ppc64le" "arm64" | ||
HELM_ARCHES:="amd64" "ppc64le" "arm64" | ||
SCORECARD_TEST_ARCHES:="amd64" "ppc64le" "arm64" | ||
SCORECARD_TEST_KUTTL_ARCHES:="amd64" "ppc64le" "arm64" | ||
ANSIBLE_ARCHES:="amd64" "ppc64le" "s390x" "arm64" | ||
HELM_ARCHES:="amd64" "ppc64le" "s390x" "arm64" | ||
CUSTOM_SCORECARD_TESTS_ARCHES:="amd64" "ppc64le" "s390x" "arm64" | ||
SCORECARD_TEST_ARCHES:="amd64" "ppc64le" "s390x" "arm64" | ||
SCORECARD_TEST_KUTTL_ARCHES:="amd64" "ppc64le" "s390x" "arm64" | ||
|
||
export CGO_ENABLED:=0 | ||
.DEFAULT_GOAL:=help | ||
|
@@ -180,7 +183,7 @@ build/%.asc: ## Create release signatures for operator-sdk release binaries | |
|
||
image: image-build image-push ## Build and push all images | ||
|
||
image-build: image-build-ansible image-build-helm image-build-scorecard-test image-build-scorecard-test-kuttl## Build all images | ||
image-build: image-build-ansible image-build-helm image-build-scorecard-test image-build-scorecard-test-kuttl image-build-custom-scorecard-tests ## Build all images | ||
|
||
image-push: image-push-ansible image-push-helm image-push-scorecard-test ## Push all images | ||
|
||
|
@@ -217,9 +220,15 @@ image-push-helm-multiarch: | |
# Scorecard test image scaffold/build/push. | ||
.PHONY: image-build-scorecard-test image-push-scorecard-test image-push-scorecard-test-multiarch | ||
|
||
# Scorecard custom test image scaffold/build/push. | ||
.PHONY: image-build-custom-scorecard-tests image-push-custom-scorecard-tests image-push-custom-scorecard-tests-multiarch | ||
|
||
# Scorecard test kuttl image scaffold/build/push. | ||
.PHONY: image-build-scorecard-test-kuttl image-push-scorecard-test-kuttl image-push-scorecard-test-kuttl-multiarch | ||
|
||
image-build-custom-scorecard-tests: | ||
./hack/image/build-custom-scorecard-tests-image.sh $(CUSTOM_SCORECARD_TESTS_BASE_IMAGE):dev | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IHMO we should not add more images since we have a lot already just for the tests. We could provide the code and doc how the users can build and push for their registry instead. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @joelanford Joe should probably decide if this example image lives inside this repo or outside in a separate repo (e.g. operator-sdk-samples). I'm fine either way. If its outside the repo then it will likely require on-going maintenance or some other external test to insure it does not get stale. |
||
image-build-scorecard-test: | ||
./hack/image/build-scorecard-test-image.sh $(SCORECARD_TEST_BASE_IMAGE):dev | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
entries: | ||
- description: Updated custom scorecard test example | ||
kind: change | ||
breaking: false | ||
migration: | ||
header: Update custom scorecard test example | ||
body: | | ||
The custom-scorecard-tests image is built like the other | ||
scorecard images with this change, the Makefile is updated | ||
to produce the custom-scorecard-tests image. This makes it | ||
easier to build the image using the normal build process and | ||
will make testing the custom image easier, which will be | ||
addressed in another PR. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would this affect the users? if not, I do not think that we need the fragment. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this PR doesn't impact users yet, another forthcoming PR however will make a change to the website docs to update some links to the new relocated file locations, so I'll add the changelog into that PR. |
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
entries: | ||
- description: Updated scorecard-test-kuttl image to use latest kuttl | ||
kind: change | ||
breaking: false | ||
migration: | ||
header: Update scorecard-test-kuttl image to use latest kuttl | ||
body: | | ||
The scorecard-test-kuttl image is updated to use kuttl:v0.5.2 | ||
as the base image. | ||
Comment on lines
+5
to
+9
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is a migration really required? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hmmn, actually that fragment is already merged into master, the way I understood migration was that it would require a change on the user's part to do something, e.g. like migrate their kuttl tests to the new image version. But I could have misunderstood the intent of the migration section. |
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
#!/usr/bin/env bash | ||
|
||
set -eux | ||
|
||
source hack/lib/image_lib.sh | ||
|
||
# build scorecard test image | ||
WD="$(dirname "$(pwd)")" | ||
GOOS=linux CGO_ENABLED=0 \ | ||
go build \ | ||
-gcflags "all=-trimpath=${WD}" \ | ||
-asmflags "all=-trimpath=${WD}" \ | ||
-o images/custom-scorecard-tests/custom-scorecard-tests \ | ||
images/custom-scorecard-tests/cmd/test/main.go | ||
|
||
# Build base image | ||
pushd images/custom-scorecard-tests | ||
docker build -t "$1" . | ||
# If using a kind cluster, load the image into all nodes. | ||
load_image_if_kind "$1" | ||
popd |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,14 +4,14 @@ metadata: | |
name: config | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The testdata/ dir has a special meaning for GO and IHMO all examples should be in the testdata dir. |
||
stages: | ||
- tests: | ||
- image: quay.io/username/custom-scorecard-tests:dev | ||
- image: quay.io/operator-framework/custom-scorecard-tests:dev | ||
entrypoint: | ||
- custom-scorecard-tests | ||
- customtest1 | ||
labels: | ||
suite: custom | ||
test: customtest1 | ||
- image: quay.io/username/custom-scorecard-tests:dev | ||
- image: quay.io/operator-framework/custom-scorecard-tests:dev | ||
entrypoint: | ||
- custom-scorecard-tests | ||
- customtest2 | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,9 +22,6 @@ import ( | |
|
||
scapiv1alpha3 "github.com/operator-framework/api/pkg/apis/scorecard/v1alpha3" | ||
apimanifests "github.com/operator-framework/api/pkg/manifests" | ||
|
||
"github.com/operator-framework/operator-sdk/internal/scorecard" | ||
"github.com/operator-framework/operator-sdk/internal/scorecard/examples/custom-scorecard-tests/internal/tests" | ||
) | ||
|
||
// This is the custom scorecard test example binary | ||
|
@@ -36,14 +33,16 @@ import ( | |
// this binary to run various tests all from within a single | ||
// test image. | ||
|
||
const PodBundleRoot = "/bundle" | ||
|
||
func main() { | ||
entrypoint := os.Args[1:] | ||
if len(entrypoint) == 0 { | ||
log.Fatal("Test name argument is required") | ||
} | ||
|
||
// Read the pod's untar'd bundle from a well-known path. | ||
cfg, err := apimanifests.GetBundleFromDir(scorecard.PodBundleRoot) | ||
cfg, err := apimanifests.GetBundleFromDir(PodBundleRoot) | ||
if err != nil { | ||
log.Fatal(err.Error()) | ||
} | ||
|
@@ -53,10 +52,10 @@ func main() { | |
// Names of the custom tests which would be passed in the | ||
// `operator-sdk` command. | ||
switch entrypoint[0] { | ||
case tests.CustomTest1Name: | ||
result = tests.CustomTest1(cfg) | ||
case tests.CustomTest2Name: | ||
result = tests.CustomTest2(cfg) | ||
case CustomTest1Name: | ||
result = CustomTest1(cfg) | ||
case CustomTest2Name: | ||
result = CustomTest2(cfg) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we have just 2 custom tests? I did not get this part. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is just an example of a custom image, it has 2 tests that essentially are no-op's to just show the structure of what a custom image looks like, real custom images that an ISV would build would have any number of tests in reality. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it has 2 only as an example....a custom image can support any number of tests. |
||
default: | ||
result = printValidTests() | ||
} | ||
|
@@ -78,10 +77,52 @@ func printValidTests() scapiv1alpha3.TestStatus { | |
result.Suggestions = make([]string, 0) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IHMO all code in the internal/ dir should be related to an internal implementation of the CLI features. We should not add examples inside that since they are not really internal. |
||
|
||
str := fmt.Sprintf("Valid tests for this image include: %s %s", | ||
tests.CustomTest1Name, | ||
tests.CustomTest2Name) | ||
CustomTest1Name, | ||
CustomTest2Name) | ||
result.Errors = append(result.Errors, str) | ||
return scapiv1alpha3.TestStatus{ | ||
Results: []scapiv1alpha3.TestResult{result}, | ||
} | ||
} | ||
|
||
const ( | ||
CustomTest1Name = "customtest1" | ||
CustomTest2Name = "customtest2" | ||
) | ||
|
||
// Define any operator specific custom tests here. | ||
// CustomTest1 and CustomTest2 are example test functions. Relevant operator specific | ||
// test logic is to be implemented in similarly. | ||
|
||
func CustomTest1(bundle *apimanifests.Bundle) scapiv1alpha3.TestStatus { | ||
r := scapiv1alpha3.TestResult{} | ||
r.Name = CustomTest1Name | ||
r.State = scapiv1alpha3.PassState | ||
r.Errors = make([]string, 0) | ||
r.Suggestions = make([]string, 0) | ||
almExamples := bundle.CSV.GetAnnotations()["alm-examples"] | ||
if almExamples == "" { | ||
fmt.Println("no alm-examples in the bundle CSV") | ||
} | ||
|
||
return wrapResult(r) | ||
} | ||
|
||
func CustomTest2(bundle *apimanifests.Bundle) scapiv1alpha3.TestStatus { | ||
r := scapiv1alpha3.TestResult{} | ||
r.Name = CustomTest2Name | ||
r.State = scapiv1alpha3.PassState | ||
r.Errors = make([]string, 0) | ||
r.Suggestions = make([]string, 0) | ||
almExamples := bundle.CSV.GetAnnotations()["alm-examples"] | ||
if almExamples == "" { | ||
fmt.Println("no alm-examples in the bundle CSV") | ||
} | ||
return wrapResult(r) | ||
} | ||
|
||
func wrapResult(r scapiv1alpha3.TestResult) scapiv1alpha3.TestStatus { | ||
return scapiv1alpha3.TestStatus{ | ||
Results: []scapiv1alpha3.TestResult{r}, | ||
} | ||
} |
This file was deleted.
This file was deleted.
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we adding s390x back in? @joelanford thoughts? Travis support did say things should be working again.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IHMO we need at least add a comment to clarifies that these images are just examples for the doc and are not required for the tool or its features work at all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed the 390x from the ARCHES to match what is on master right now, also added a comment in the makefile about the scorecard custom image being an example.