Build, tag, and push operator base images during CI #832

joelanford · 2018-12-10T17:19:13Z

Description of the change:

Adding new Makefile targets and scripts to build, tag, and push docker images for the Ansible and Helm operators
- The tagging logic is:
  - Always tag with $TRAVIS_BRANCH (which is set to $TRAVIS_TAG for tagged commits)
  - Also tag with latest if $TRAVIS_TAG is set and it matches the latest semver version found in the repo.
Changing builds and scaffolds to use quay.io/operator-framework instead of quay.io/water-hole
- Forked repositories can set ANSIBLE_IMAGE and HELM_IMAGE in their travis repository settings to override the full image name used when pushing to a docker repository.
Updating .travis.yml with a new deploy stage, which is only triggered when both of the following conditions are true:
- The commit is not a pull request
- Commit is tagged OR Branch is master OR the commit message contains [travis deploy]

Motivation for the change:
Our CI process needs to automatically build, tag, and deploy our base images for the Ansible and Helm operators.

NOTE: Because the deploy stage does not run during PR builds, we won't see the new deploy stage in the travis builds for this PR. Here's a link to the build for my fork: https://travis-ci.org/joelanford/operator-sdk/builds/466080812

…argets

…r-framework

…ocker images

joelanford · 2018-12-10T17:22:41Z

@hasbro17 Before we merge this, we need a robot account created in quay.io/operator-framework with write permission on ansible-operator and helm-operator (looks like only owners can do this in quay), and then DOCKER_USERNAME and DOCKER_PASSWORD set with the robot account credentials as secure environment variables in Travis.

UPDATE: @shawn-hurley was able to do this for me. Thanks Shawn!

Makefile

…age'

joelanford · 2018-12-10T20:16:23Z

Latest CI: https://travis-ci.org/joelanford/operator-sdk/builds/466162172

joelanford · 2018-12-11T21:14:26Z

Also, looks like this will close #555.

AlexNPavel

Looks great

estroz

TIL a lot about travis directives 😁

lilic

lgtm overall, nice work 👍

Few suggestions and questions.

.travis.yml

Makefile

hack/image/build-ansible-image.sh

hack/image/build-helm-image.sh

…back

joelanford · 2018-12-12T15:55:19Z

Latest build: https://travis-ci.org/joelanford/operator-sdk/builds/467050239

joelanford · 2018-12-13T15:46:14Z

hack/image/build-ansible-image.sh

+set -eux
+
+# build operator binary and base image
+go build -o test/ansible-operator/ansible-operator test/ansible-operator/cmd/ansible-operator/main.go


@shawn-hurley @mhrivnak I'm guessing test/ansible-operator/cmd/ansible-operator/main.go is not (yet, at least) what we want for the official ansible-operator base image?

Can we pull in the latest changes from the main.go in github.com/water-hole/ansible-operator or do we need to wait on #823?

I think we can pull in the latest from water-hole I don't see any reason not too. Once this change goes in the this becomes our main file I believe.

Personally, I would like to see these at a new location that is not hidden under the test dir. I think that we can deal with that stuff much later, just throwing out the concern.

@shawn-hurley For the scorecard PR, I have the image for the scorecard-proxy in a new images folder. I think it would be a good idea to move our base images into a specific directory like that. We could make another PR after this gets merged to move the base images to a more reasonable location.

@shawn-hurley I reverted my changes to test/ansible-operator/cmd/ansible-operator/main.go that were meant to match it up water-hole. Now with #754, the current version looks like what we want. WDYT?

No, we need these changes https://github.com/water-hole/ansible-operator/blob/master/cmd/manager/main.go#L38-L44

for the namespace

…opying from github.com/water-hole/ansible-operator

joelanford · 2018-12-13T21:29:36Z

Latest CI with updated ansible-operator main.go: https://travis-ci.org/joelanford/operator-sdk/builds/467668311

…duction logger

joelanford · 2018-12-14T18:58:07Z

pkg/scaffold/ansible/dockerfile.go

@@ -39,7 +39,7 @@ func (d *Dockerfile) GetInput() (input.Input, error) {
 	return d.Input, nil
 }

-const dockerFileAnsibleTmpl = `FROM quay.io/water-hole/ansible-operator
+const dockerFileAnsibleTmpl = `FROM quay.io/operator-framework/ansible-operator


In the Dockerfile scaffolds for helm and ansible, should we pin to an image tag? Something like the following?

d.ImageTag = strings.TrimSuffix(version.Version, "+git")

And

const dockerFileAnsibleTmpl = `FROM quay.io/operator-framework/ansible-operator:{{.ImageTag}} ... `

If so, we'll have to be careful to make sure our e2e tests use the just-built image vs an existing tagged one.

Good question. Our e2e tests will definitely need to build a dev ansible/helm base image from the PR before testing.
For our tagged releases for the SDK we definitely want to use ansible/helm base images with the same image tag as the SDK release version in the scaffold.

But when using the SDK on the master branch, do we want to scaffold base images for the last semver release, or use base images built from the master. I think that we'll need to do the latter.

Right now it seems like we don't have a release tag so it will always use the latest updated tag.

I agree, if we're testing a PR on master we should build base images from master. We might want to add a step in the release process in another PR for building, tagging, and pushing ansible and helm base images as per @shawn-hurley's previous comment and #555.

I actually think that the tag should just be the release that is for that operator-sdk binary. I wouldn't add the git because it should be the release number I think right?

Here are my thoughts:

During CI, we can use whatever tag we want with the CI-built image. Ideally we name it something that we never actually push to avoid accidentally using an image we don't intend to. I propose :dev. So no matter what branch or tag Travis is building, we'll build :dev as our base image locally.

Next, during the e2e tests, we want to use that :dev image. So, after running operator-sdk new but before operator-sdk build, I would suggest always replacing the scaffolded image name in build/Dockerfile with quay.io/operator-framework/ansible-operator:dev

Finally, when we go to push the image tags, we'll run the following for each $PUSH_TAG:
docker tag quay.io/operator-framework/ansible-operator:dev quay.io/operator-framework/ansible-operator:$PUSH_TAG docker push quay.io/operator-framework/ansible-operator:$PUSH_TAG

As for what image tag to scaffold, it sounds like we agree that if version.Version does not end with +git, then we'll use version.Version. The biggest question in my mind is what to scaffold when it does contain +git. The options in my mind are:

Use :master.

Pro: If users are running operator-sdk from master, they'll get the most recent base image changes.

Con: If users are running operator-sdk from an unreleased commit prior to the last stable release, it's possible that the master base image will be incompatible with the rest of the scaffolded operator.

Just drop +git.

Pro: No matter what build of operator-sdk is being used, it's likely that the base image will work, since we'll use the "closest" one.

Con: If using operator-sdk from master, the latest base image changes will not be present.

Use a broken placeholder, like REPLACE_TAG:

In this case, the operator will fail to build, forcing the decision onto the user.

^^ It will point to the latest release.

Also, I'd suggest :master for a couple of reasons.

It's more obvious to users where it came from.

Because it just comes from the branch name, it makes the scripts more flexible. If we wanted to started pushing base images for another branch (say we have another big integration effort like controller-runtime), it would be as easy as updating the if: condition in the .travis.yml deploy stage.

@joelanford @hasbro17 I personally think that a new ansible or helm operator always uses the latest release version (i.e 0.3.0), not master. I think that if a user wants to use latest or master that is an explicit change to use it in their dockerfile.

That sounds fine to me. So long as we maintain a :master tag it's not too much work for users to use the :master base image when they want to.

@joelanford WDYT? We always keep the :latest tag pointing to the latest release.
And in the scaffold we always use the latest release tag e.g :v0.3.0. (Can't use :latest I think because that can break older projects when we make a new release image and change latest).

Agreed. I'll update the PR with this change.

…tches from env, not hardcoded file

shawn-hurley

I don't really understand the Travis file or how it is working. I have yet to see the << tag or the &go_before_install and *go_before_install

shawn-hurley · 2018-12-17T19:54:29Z

hack/image/build-ansible-image.sh

+# build operator binary and base image
+go build -o test/ansible-operator/ansible-operator test/ansible-operator/cmd/ansible-operator/main.go
+pushd test/ansible-operator
+docker build -t "$1" .


Shouldn't we dogfood our stuff here, and just use operator-sdk build.

Probably. For this PR, I just moved these lines directly out of the e2e-ansible.sh script to make them reusable.

I think this is related to your comment about where the main functions are stored. Is it possible to use operator-sdk build with the current repo layout?

I would expect that this script will be changed or reorganized based on the outcome of @mhrivnak's upcoming proposal to use the operator-sdk binary directly and/or the migrate command from #823 .

shawn-hurley · 2018-12-17T20:02:03Z

pkg/scaffold/ansible/dockerfile.go

@@ -39,7 +39,7 @@ func (d *Dockerfile) GetInput() (input.Input, error) {
 	return d.Input, nil
 }

-const dockerFileAnsibleTmpl = `FROM quay.io/water-hole/ansible-operator
+const dockerFileAnsibleTmpl = `FROM quay.io/operator-framework/ansible-operator


I actually think that the tag should just be the release that is for that operator-sdk binary. I wouldn't add the git because it should be the release number I think right?

joelanford · 2018-12-17T20:50:26Z

I don't really understand the Travis file or how it is working. I have yet to see the << tag or the &go_before_install and *go_before_install

@shawn-hurley This syntax is using YAML merge keys (documented here). It's basically a way of declaring re-usable YAML maps and then merging them into other maps, enabling composition and reuse.

Since our travis build has multiple concerns now, I figured this would be the easiest way to maintain it without repeating ourselves or performing unnecessary steps unrelated to the CI task at hand (e.g. we almost always need to install and run dep, but we don't need to install ansible for the Helm operator base image deploy stage).

I'm definitely open to suggestions though. I don't have a whole lot of experience with Travis.

… local-only image tag

…s from latest release

joelanford · 2018-12-18T18:05:52Z

Latest CI build: https://travis-ci.org/joelanford/operator-sdk/builds/469623064

…atch all namespaces if WATCH_NAMESPACE is unset

…nto push-images

…ixing ansible-operator compile error

joelanford · 2018-12-18T20:26:51Z

Latest CI build: https://travis-ci.org/joelanford/operator-sdk/builds/469682451

hasbro17

LGTM

Nice work! Let's see if it all comes together by testing out the master branch image build after this gets merged.

hasbro17 · 2018-12-18T21:24:27Z

@joelanford I always forget but can you please follow this up to update the CHANGELOG as well.
We need to point out that ansible/helm base images now use quay.io/operator-framework instead of quay.io/water-hole.
That'll go in the CHANGED section.

joelanford · 2018-12-18T21:39:01Z

@hasbro17 #862

…rk#832) * Makefile,hack/docker,hack/tests: adding new docker scripts and Make targets * Makefile,pkg/scaffold: changing default image repo to quay.io/operator-framework * [travis deploy] .travis.yml: refactor into stages to build and push docker images * [travis deploy] Makefile,hack/: renaming make 'docker' targets to 'image' * [travis deploy] .travis.yml: renaming make 'docker' targets to 'image' * [travis deploy] .travis.yml,Makefile,hack/image: updates from PR feedback * [travis deploy] test/ansible-operator/cmd/ansible-operator/main.go: copying from github.com/water-hole/ansible-operator * [travis deploy] test/helm-operator/cmd/helm-operator/main.go: use production logger * [travis deploy] test/helm-operator/cmd/helm-operator/main.go: load watches from env, not hardcoded file * [travis deploy] Makefile,hack/tests: always build, test, and tag from local-only image tag * [travis deploy] pkg/scaffold/ansible,pkg/scaffold/helm: use image tags from latest release * [travis deploy] Makefile: add missing PHONY target for test/ci-helm * [travis deploy] test/ansible-operator/cmd/ansible-operator/main.go: watch all namespaces if WATCH_NAMESPACE is unset * [travis deploy] test/ansible-operator/cmd/ansible-operator/main.go: fixing ansible-operator compile error

joelanford added 3 commits December 10, 2018 11:35

Makefile,hack/docker,hack/tests: adding new docker scripts and Make t…

a347fc5

…argets

Makefile,pkg/scaffold: changing default image repo to quay.io/operato…

d59e1a2

…r-framework

[travis deploy] .travis.yml: refactor into stages to build and push d…

0b29939

…ocker images

openshift-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Dec 10, 2018

estroz reviewed Dec 10, 2018

View reviewed changes

Makefile Outdated Show resolved Hide resolved

joelanford added 2 commits December 10, 2018 12:46

[travis deploy] Makefile,hack/: renaming make 'docker' targets to 'im…

a4600e7

…age'

[travis deploy] .travis.yml: renaming make 'docker' targets to 'image'

284638b

joelanford requested review from AlexNPavel, mhrivnak, hasbro17, lilic and shawn-hurley December 10, 2018 21:07

AlexNPavel approved these changes Dec 11, 2018

View reviewed changes

estroz approved these changes Dec 12, 2018

View reviewed changes

lilic approved these changes Dec 12, 2018

View reviewed changes

.travis.yml Outdated Show resolved Hide resolved

Makefile Outdated Show resolved Hide resolved

Makefile Outdated Show resolved Hide resolved

hack/image/build-ansible-image.sh Show resolved Hide resolved

hack/image/build-helm-image.sh Show resolved Hide resolved

[travis deploy] .travis.yml,Makefile,hack/image: updates from PR feed…

43112f8

…back

joelanford force-pushed the push-images branch from f70d8b9 to 43112f8 Compare December 12, 2018 15:23

joelanford commented Dec 13, 2018

View reviewed changes

joelanford force-pushed the push-images branch from c721504 to 5f181fc Compare December 13, 2018 19:02

[travis deploy] test/ansible-operator/cmd/ansible-operator/main.go: c…

f1a1459

…opying from github.com/water-hole/ansible-operator

joelanford force-pushed the push-images branch from 5f181fc to f1a1459 Compare December 13, 2018 19:17

[travis deploy] test/helm-operator/cmd/helm-operator/main.go: use pro…

b1968aa

…duction logger

joelanford force-pushed the push-images branch from 932fb14 to b1968aa Compare December 14, 2018 18:39

joelanford commented Dec 14, 2018

View reviewed changes

[travis deploy] test/helm-operator/cmd/helm-operator/main.go: load wa…

68abf3c

…tches from env, not hardcoded file

joelanford mentioned this pull request Dec 17, 2018

helm/: added helm-based memcached sample operator operator-framework/operator-sdk-samples#51

Merged

shawn-hurley requested changes Dec 17, 2018

View reviewed changes

shawn-hurley approved these changes Dec 17, 2018

View reviewed changes

joelanford added 4 commits December 18, 2018 10:38

Merge branch 'master' into push-images

0638711

[travis deploy] Makefile,hack/tests: always build, test, and tag from…

a32fee2

… local-only image tag

[travis deploy] pkg/scaffold/ansible,pkg/scaffold/helm: use image tag…

0fcd0ce

…s from latest release

[travis deploy] Makefile: add missing PHONY target for test/ci-helm

7b38800

joelanford force-pushed the push-images branch from 96ad2f8 to 7b38800 Compare December 18, 2018 17:27

[travis deploy] test/ansible-operator/cmd/ansible-operator/main.go: w…

32c59dd

…atch all namespaces if WATCH_NAMESPACE is unset

openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 18, 2018

Merge branch 'master' of github.com:operator-framework/operator-sdk i…

fc58ccf

…nto push-images

openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 18, 2018

[travis deploy] test/ansible-operator/cmd/ansible-operator/main.go: f…

21bedd4

…ixing ansible-operator compile error

hasbro17 approved these changes Dec 18, 2018

View reviewed changes

joelanford merged commit 3a8d517 into operator-framework:master Dec 18, 2018

joelanford deleted the push-images branch December 18, 2018 22:04

joelanford mentioned this pull request Dec 19, 2018

Release ansible base image/binary #555

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Build, tag, and push operator base images during CI #832

Build, tag, and push operator base images during CI #832

joelanford commented Dec 10, 2018

joelanford commented Dec 10, 2018 •

edited

Loading

joelanford commented Dec 10, 2018

joelanford commented Dec 11, 2018

AlexNPavel left a comment

estroz left a comment

lilic left a comment

joelanford commented Dec 12, 2018

joelanford Dec 13, 2018

shawn-hurley Dec 13, 2018

AlexNPavel Dec 13, 2018

joelanford Dec 18, 2018 •

edited

Loading

shawn-hurley Dec 18, 2018

joelanford commented Dec 13, 2018

joelanford Dec 14, 2018 •

edited

Loading

hasbro17 Dec 17, 2018

estroz Dec 17, 2018 •

edited

Loading

shawn-hurley Dec 17, 2018

joelanford Dec 17, 2018

joelanford Dec 17, 2018

joelanford Dec 17, 2018 •

edited

Loading

shawn-hurley Dec 18, 2018

hasbro17 Dec 18, 2018

joelanford Dec 18, 2018

shawn-hurley left a comment

shawn-hurley Dec 17, 2018

joelanford Dec 17, 2018

shawn-hurley Dec 17, 2018

joelanford commented Dec 17, 2018

joelanford commented Dec 18, 2018

joelanford commented Dec 18, 2018

hasbro17 left a comment

hasbro17 commented Dec 18, 2018

joelanford commented Dec 18, 2018

Build, tag, and push operator base images during CI #832

Build, tag, and push operator base images during CI #832

Conversation

joelanford commented Dec 10, 2018

joelanford commented Dec 10, 2018 • edited Loading

joelanford commented Dec 10, 2018

joelanford commented Dec 11, 2018

AlexNPavel left a comment

Choose a reason for hiding this comment

estroz left a comment

Choose a reason for hiding this comment

lilic left a comment

Choose a reason for hiding this comment

joelanford commented Dec 12, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joelanford Dec 18, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joelanford commented Dec 13, 2018

joelanford Dec 14, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

estroz Dec 17, 2018 • 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

joelanford Dec 17, 2018 • 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

shawn-hurley left a 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

joelanford commented Dec 17, 2018

joelanford commented Dec 18, 2018

joelanford commented Dec 18, 2018

hasbro17 left a comment

Choose a reason for hiding this comment

hasbro17 commented Dec 18, 2018

joelanford commented Dec 18, 2018

joelanford commented Dec 10, 2018 •

edited

Loading

joelanford Dec 18, 2018 •

edited

Loading

joelanford Dec 14, 2018 •

edited

Loading

estroz Dec 17, 2018 •

edited

Loading

joelanford Dec 17, 2018 •

edited

Loading