Skip to content
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

Scorecard2 kuttl image #3278

Merged
merged 28 commits into from
Jul 6, 2020
Merged

Scorecard2 kuttl image #3278

merged 28 commits into from
Jul 6, 2020

Conversation

jmccormick2001
Copy link
Contributor

Description of the change:
this PR adds the scorecard-test-kuttl image

this image is to support kuttl integration into scorecard2 allowing scorecard users to
add and execute kuttl tests via scorecard2. Follow on PRs will modify the travis
configuration to build this image as part of a release...a follow on PR will also add
user facing web docs regarding kuttl tests.

Motivation for the change:

part of the overall scorecard2 implementation.

@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 23, 2020
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 23, 2020
@@ -32,16 +32,19 @@ ANSIBLE_BASE_IMAGE = quay.io/operator-framework/ansible-operator
HELM_BASE_IMAGE = quay.io/operator-framework/helm-operator
SCORECARD_PROXY_BASE_IMAGE = quay.io/operator-framework/scorecard-proxy
SCORECARD_TEST_BASE_IMAGE = quay.io/operator-framework/scorecard-test
SCORECARD_TEST_KUTTL_BASE_IMAGE = quay.io/operator-framework/scorecard-test-kuttl
Copy link
Member

Choose a reason for hiding this comment

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

We've got to start consolidating these image builds. We have a ton of deploy jobs on merge with master. For 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.

IMO we need have the targets to run/exec each one if we just want a specific image. However, I agree that the setup in the Travis master now has a lot of images and we could track a task to try aggregate then if it will reduce the time.

func (Plugin) Name() string { return (kbgov2.Plugin{}).Name() }
func (Plugin) Version() plugin.Version { return (kbgov2.Plugin{}).Version() }
func (Plugin) SupportedProjectVersions() []string { return (kbgov2.Plugin{}).SupportedProjectVersions() }
func (Plugin) Name() string {
Copy link
Member

Choose a reason for hiding this comment

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

I assume these gofmt changes are a result of using go v1.14. These changes should be made separately, and we should consider bumping the SDK's go version to v1.14.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I switched this back to the original format. Not sure but it must have been my local dev setup.

Copy link
Contributor

Choose a reason for hiding this comment

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

If you are using the latest version of the lint and then., run make lint-fix it will fix the layout in the way that works for both versions. I have been sofer resting the changes in this file. Could we please approve #3270 ?

c/c @jmrodri

Copy link
Member

@estroz estroz left a comment

Choose a reason for hiding this comment

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

On a first pass this LGTM, need to do a more in-depth review. Also, iirc the user_setup and entrypoint scripts are no longer needed.

@jmccormick2001
Copy link
Contributor Author

/retest


image-push-scorecard-test-kuttl-multiarch:
./hack/image/push-manifest-list.sh $(SCORECARD_TEST_KUTTL_IMAGE) ${SCORECARD_TEST_KUTTL_ARCHES}

Copy link
Contributor

Choose a reason for hiding this comment

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

is not missing call it in the travis? See: https://github.com/operator-framework/operator-sdk/blob/master/.travis.yml#L319-L323

Also, have we already the images repo configured and etc ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

repos are ready for this but I was going to do the travis update in a separate PR.

Copy link
Contributor

@camilamacedo86 camilamacedo86 left a comment

Choose a reason for hiding this comment

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

It shows for me:
/lgtm /approve

PS.: Travis update for master build/push the images will be done in a follow-up. see https://github.com/operator-framework/operator-sdk/pull/3278/files#r449917308

Copy link
Member

@jmrodri jmrodri left a comment

Choose a reason for hiding this comment

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

/lgtm

One nit but that can be fixed at a later time or if there are other more present changes.

@@ -156,7 +159,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-proxy image-build-scorecard-test ## Build all images
image-build: image-build-ansible image-build-helm image-build-scorecard-proxy image-build-scorecard-test image-build-scorecard-test-kuttl## Build all images
Copy link
Member

Choose a reason for hiding this comment

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

nit: should be a space before the ## Build all images.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks, will fix that in an upcoming PR to update the SHA image of the kuttl base image.

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 6, 2020
@jmccormick2001 jmccormick2001 merged commit 747913d into operator-framework:master Jul 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants