*: use CI-built scorecard images during e2e tests

[joelanford]

Description of the change:
Use CI-built scorecard images during e2e tests

Motivation for the change:
Right now, we use either master or the latest release image during CI. However, this has 2 problems:

1. It doesn't test any changes to these images that may have happened in PRs
2. It fails during releases PRs because the release image is not yet built and pushed.

This is related to #3845.

[joelanford]

This is required to be backported to v1.0.x so that the v1.0.1 release PR will pass in #3901.

[camilamacedo86]

@joelanford these are not the only places. The images are scaffold in many files in the sub dir of config and bundle. If you check the Memcached Go Sample you will see that it needs to be changed in:

• /scorecard/config.yaml
• /patches/olm.config.yaml
• /patches/basic.config.yaml

However, it shows changed from 1.0.0 to now. So, if you run the test and ad a breaking point you will find the places in the bundle which are not in the Memcached sample.

IMO we could to do:

By("replacing scorecard-test image to use the dev tag")
const scorecardImage :="quay.io/operator-framework/scorecard-test:master"
const scorecardImageReplace :="quay.io/operator-framework/scorecard-test:dev"
err = testutils.ReplaceInAllFilesFromTree(filepath.Join(tc.Dir, "bundle"), scorecardImage,scorecardImageReplace)
Expect(err).NotTo(HaveOccurred())
err = testutils.ReplaceInAllFilesFromTree(filepath.Join(tc.Dir, "config", "scorecard"), scorecardImage, scorecardImageReplace)
Expect(err).NotTo(HaveOccurred())

// ReplaceInAllFiles replaces all instances of old with new in the files of a directory and its subdirectories.
func ReplaceInAllFilesFromTree(projectPath, old, new string) error {
	err := filepath.Walk(projectPath,
		func(path string, info os.FileInfo, err error) error {
			if err != nil {
				return err
			}
			if !info.IsDir() {
				ReplaceInFile(path, old, new)
			}
			return nil
		})
	if err != nil {
		return err
	}
	return nil
}

[joelanford]

As I understand it, we don't need to replace in ./bundle, because that directory is populated from the contents of the config directory when make bundle is run. So the changes we make in the kustomize patch files propagate to the bundle.

[camilamacedo86]

HI @joelanford,

Regards the bundle it makes sense only if the bundle is generated after this replace. Note that in the future we will build the bundle before in the samples and then, we will do this replace just for the e2e tests.

See that it shows still missing the /scorecard/config.yaml.

Also, note that it is very easy we add new scaffolds and just forget that we need to replace the images. So, I think it is better we add the func that will replace ALL in a tree as suggested above for we do not end up by mistake in this situation.

[joelanford]

Note that in the future we will build the bundle before in the samples and then, we will do this replace just for the e2e tests.

This PR is a minimal change to get this working. I plan to backport this to v1.0.1, so I don't think it is a good idea to make the bigger change that you're suggesting for a backported patch. Let's capture this subject as something to solve in your enhancement for generating the samples.

See that it shows still missing the /scorecard/config.yaml.

As far as I can tell, this file doesn't exist in the e2e project (or in the operator-sdk-samples project) and all references correctly use the dev image tag. Am I missing something somewhere?

$ cd e2e-kbaf
$ grep -r 'image:.*scorecard-test' * | sort | uniq
bundle/tests/scorecard/config.yaml:    image: quay.io/operator-framework/custom-scorecard-tests:dev
bundle/tests/scorecard/config.yaml:    image: quay.io/operator-framework/scorecard-test:dev
config/scorecard/patches/basic.config.yaml:    image: quay.io/operator-framework/scorecard-test:dev
config/scorecard/patches/custom.config.yaml:    image: quay.io/operator-framework/custom-scorecard-tests:dev
config/scorecard/patches/olm.config.yaml:    image: quay.io/operator-framework/scorecard-test:dev

[camilamacedo86]

Few nits @joelanford.

• It is not replacing all manifests for the GO test that will have the image
• Ansible/Helm e2e tests also require to be updated as well.
• You are facing the same issue faced by me: Error: error running tests context deadline exceeded in wip - Ensure that all scorecard tests are checked and with changes made in the PR #3843

[joelanford]

Updated to add the same replacements for the helm and ansible e2e runs.

@camilamacedo86, I think the issue both of us hit is that the dev images were not actually being loaded into the kind cluster because the kubectl command was failing during load_image_if_kind. See https://travis-ci.org/github/operator-framework/operator-sdk/jobs/728156932#L1227

[jmrodri]

looks okay to me. I'll wait for @camilamacedo86 to be able to respond before lgtm'ing

[camilamacedo86]

See my comment in: https://github.com/operator-framework/operator-sdk/pull/3909/files#r490877131

• It shows still missing a file to be updated /scorecard/config.yaml
• in order to improve the maintainability and avoid the scenario where we add more manifests and forget to replace the images I'd advocate in favour of we replace ALL files in the tree instead of each one. IMO it will ensure that behaviour after we change it to use the gen samples instead.
• Nit: WDYT about we have a helper func in test/internal/ such as : ReplaceAllScorecardImagesForDev() to centralize the code

Otherwise, it shows /lgtm for me.
Btw really tks for discovery the root cause of the issue faced by us :-)

[joelanford]

Nit: WDYT about we have a helper func in test/internal/ such as : ReplaceAllScorecardImagesForDev() to centralize the code

Probably a good idea. I'll capture in a separate issue for follow-up.

EDIT: #3912