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

Automate Container Image Builds Using GCB #217

Merged

Conversation

seanmalloy
Copy link
Member

Adds a GCB(Google Cloud Build) configuraiton file that automates the
building and pushing of descheduler container images to the official
Google Container Registry for k8s.

Reference documentation:
https://github.com/kubernetes/test-infra/blob/master/config/jobs/image-pushing/README.md

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jan 25, 2020
@k8s-ci-robot k8s-ci-robot requested review from damemi and k82cn January 25, 2020 06:43
@seanmalloy
Copy link
Member Author

This has NOT been tested, so please don't merge it yet. I think right now it will only build the container image, but won't push it. Some tweaks might need to be made to the Makefile, but not 100% sure yet.

@seanmalloy
Copy link
Member Author

/hold

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 25, 2020
Adds a GCB(Google Cloud Build) configuraiton file that automates the
building and pushing of descheduler container images to the official
Google Container Registry for k8s.

Reference documentation:
https://github.com/kubernetes/test-infra/blob/master/config/jobs/image-pushing/README.md
@seanmalloy
Copy link
Member Author

/hold cancel

I made one small change to the GCB config to also push the container image to the staging registry. I believe it will work correctly now. However it is not clear to be how to test the GCB job prior to merging this to the master branch.

@damemi @aveshagarwal @ravisantoshgudimetla this is ready for review now.

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 26, 2020
@seanmalloy
Copy link
Member Author

/assign @aveshagarwal

substitutions:
# _GIT_TAG will be filled with a git-based tag for the image, of the form vYYYYMMDD-hash, and
# can be used as a substitution
_GIT_TAG: '12345'
Copy link
Contributor

Choose a reason for hiding this comment

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

What does this mean? Will this be substituted by the git-tag v20190906-745fed4 ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Short answer ...

Yeah it looks like it will be vYYYYMMDD-GIT_TAG. For example v20200131-v0.10.0. I think this is not what we want. When a descheduler version is released the tag should be something like v0.10.0. I'll have to figure out how to change this.

Long answer ...

It looks like this a GCB user defined substitution. The container image listed on line 10 must be going something special with the variable.

I believe I tracked it down to this directory https://github.com/kubernetes/test-infra/tree/master/images/builder. More specifically here ... https://github.com/kubernetes/test-infra/blob/09a81c4adf5b662b2643dccb9d285ad3102af4af/images/builder/main.go#L48-L56.

It seems like most other k8s projects using image build automation are using _GIT_TAG. See here, here and here.

Copy link
Member Author

Choose a reason for hiding this comment

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

One more thought ...

We could merge this PR as is and sort out git/docker tag configuration is a follow up PR. This would at least enable the automated builds for the master branch and let us confirm that the build automation is working.

Also be aware that the container build automation only pushes container images the GCR staging registry. A person will still need to submit a pull request for each container image that needs to be promoted to the production GCR registry.

Choose a reason for hiding this comment

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

Another thought (which is where the design comes from): it doesn't really matter what you tag things in the staging registry - when the image promoter promotes them, they can be given a different tag to what they had in staging.

Copy link
Member Author

Choose a reason for hiding this comment

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

@Katharine thanks for adding your comment. This makes sense to me now. We don't need to worry about the tags in the staging registry.

@ravisantoshgudimetla let me know if you have any additional questions on this topic.

Copy link
Contributor

Choose a reason for hiding this comment

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

So, with this as it is we'll get staging repo tags in the format v20190906-v0.9.0-76-g30d05382b (based on my own run of the function you linked to here).

That seems good enough for me in the staging repo, it's descriptive of the version. And we choose our published tag when promoted, so it will be cleaner (like v0.9.0). Sgtm

@seanmalloy
Copy link
Member Author

/hold

I need to add some basic docs explaining how this works for both the automated and manual process.

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 29, 2020
@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jan 30, 2020
@seanmalloy
Copy link
Member Author

/hold cancel

I added commit ac089fe to this pull request that documents the new release process. This PR is now ready for review. @ravisantoshgudimetla @aveshagarwal and @damemi please review this commit to make sure the release documentation is clear. Thanks!

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 30, 2020
@damemi
Copy link
Contributor

damemi commented Jan 31, 2020

@seanmalloy thanks for this work, this all looks good to me. The instructions you linked are pretty clear and are definitely helpful to add.
/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 31, 2020
@seanmalloy seanmalloy mentioned this pull request Feb 1, 2020
6 tasks
@ravisantoshgudimetla
Copy link
Contributor

@seanmalloy - Thanks for working on this.

/lgtm

@ravisantoshgudimetla
Copy link
Contributor

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ravisantoshgudimetla, seanmalloy

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [ravisantoshgudimetla]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 3, 2020
@k8s-ci-robot k8s-ci-robot merged commit 861f057 into kubernetes-sigs:master Feb 3, 2020
@seanmalloy seanmalloy deleted the cloud-build-config branch February 7, 2020 06:05
briend pushed a commit to briend/descheduler that referenced this pull request Feb 11, 2022
…ld-config

Automate Container Image Builds Using GCB
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants