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

kep: image promoter: process to copy container images #2695

Merged
merged 1 commit into from
Nov 14, 2018

Conversation

javier-b-perez
Copy link
Contributor

KEP to create a process for publishing container images into the official kubernetes container registry.

@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. sig/architecture Categorizes an issue or PR as relevant to SIG Architecture. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Sep 20, 2018
@cblecker
Copy link
Member

/ok-to-test
/cc @ixdy @dims

@k8s-ci-robot k8s-ci-robot removed the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Sep 20, 2018
@dims
Copy link
Member

dims commented Sep 20, 2018

@javier-b-perez wanted to raise this question. As part of the promotion, do we want to sign the images? please see

@BenTheElder
Copy link
Member

Thanks @javier-b-perez! Initial pass LGTM 😄

@javier-b-perez
Copy link
Contributor Author

@javier-b-perez wanted to raise this question. As part of the promotion, do we want to sign the images? please see

That is a good point, I will investigate if this is possible and if we can, I think we should sign during the promotion process.

@thockin
Copy link
Member

thockin commented Sep 21, 2018

Idea LGTM. Who else do we want to look at this? @kubernetes/sig-release-members maybe ?

@thockin thockin self-assigned this Sep 21, 2018
@dims
Copy link
Member

dims commented Sep 21, 2018

Needs a small update after looking at signing possibilities. LGTM otherwise.

@javier-b-perez
Copy link
Contributor Author

Needs a small update after looking at signing possibilities. LGTM otherwise.

Looking at GCR seems it is not supported at the moment, but we can still push to multiple registries, if we want, (like GCR, docker hub) and sign images when possible as part of the promotion process.
I will update the text with a comment saying that as part of the promotion process, we could sign images.

@thockin
Copy link
Member

thockin commented Sep 21, 2018 via email

@tpepper
Copy link
Member

tpepper commented Sep 25, 2018

We're mulling perhaps something for a larger KEP around an improved release engineering process and possible connections to SIG Testing improvements as things transition out from under Google to CNCF.

@thockin
Copy link
Member

thockin commented Sep 25, 2018 via email

@dims
Copy link
Member

dims commented Sep 28, 2018

@tpepper what's detailed here in this KEP will be an important part of whatever we come up with for a "larger KEP around an improved release engineering". So i'd like to get this approved.

/cc @AishSundar @calebamiles @kubernetes/k8s-infra-team

Copy link

@AishSundar AishSundar left a comment

Choose a reason for hiding this comment

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

Thanks @javier-b-perez this LGTM, just had a minor comments.

keps/sig-release/0028-k8s-image-promoter.md Outdated Show resolved Hide resolved
keps/sig-release/0028-k8s-image-promoter.md Outdated Show resolved Hide resolved
@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 9, 2018
@justaugustus
Copy link
Member

/kind kep

There are multiple reasons why we should have a process to publish container images in place:

* We cannot allow all community members to publish images into the official kubernetes container registry.
* We should restrict who can push images to a small set of members and some systems accounts for automation.
Copy link
Member

Choose a reason for hiding this comment

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

How do we do this now? can we control access to the individual images vs directory?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We cannot control individual images or directories in GCR. This is why I say it should be a "small set of members".
We can restrict who can merge or approve PR into the manifest's repository, then leave the rest to the promoter tool). So those who approve PRs don't even need to have write access into the container registry.

Copy link
Member

Choose a reason for hiding this comment

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

@javier-b-perez Would it make sense to have each sub-project get a new staging area of their own, and have the promoter pull from any of those?

I guess we'd need a new GCP project for each bucket (I can't see how to make >1 bucket in a project). The promoter works on hash, so it should Just Work, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@thockin it is possible to have a per project staging container registry, that means we need to create a GCP project per each GCR we want to handle.
The promoter tool just need read access to this GCR and it can copy the container image with the same digest from project staging to official container registry.

How do you see the staging domain working? something like:
staging-k8s.gcr.io/project1/imageA -> GCR/project1/imageA
staging-k8s.gcr.io/project2/imageB -> GCR/project2/imageB
(not sure if this is possible, might not)

Copy link
Member

Choose a reason for hiding this comment

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

I'm just thinking out loud. The options are really

a) Be careful who can push because anyone who can write can do anything.

b) Give each sub-team their own space.

e.g. gcr.io/k8s-foo-staging for project foo and gcr.io/k8s-bar-staging for project bar, both promote to gcr.k8s.io/{foo,bar}/{foo,bar}:<tag>

@dims @mkumatag thoughts?

@thockin
Copy link
Member

thockin commented Oct 17, 2018 via email

@k8s-ci-robot k8s-ci-robot added sig/pm and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Nov 6, 2018
@javier-b-perez javier-b-perez force-pushed the image-promoter branch 2 times, most recently from 66aa093 to 6b767dd Compare November 14, 2018 01:06
@listx
Copy link

listx commented Nov 14, 2018

I'd like to state for the record (please let it be known!!) that I'm highly interested in creating a demo of a promotion tool to help realize this KEP. It would be responsible for performing various promotions and whatnot.

I should have a proof-of-concept ready before the end of the year.

@listx
Copy link

listx commented Nov 14, 2018

@calebamiles What will it take to get this KEP approved?

@dims
Copy link
Member

dims commented Nov 14, 2018

@listx i think we need approval from @thockin as mentioned in the proposal itself :)

@listx
Copy link

listx commented Nov 14, 2018

@dims Oops, I completely missed the metadata part.

As a reviewer myself, LGTM.

@thockin
Copy link
Member

thockin commented Nov 14, 2018

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 14, 2018
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 14, 2018
@javier-b-perez javier-b-perez force-pushed the image-promoter branch 2 times, most recently from 5a9e330 to 0ba3c71 Compare November 14, 2018 20:47
@calebamiles
Copy link
Contributor

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 14, 2018
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: calebamiles, thockin

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:

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 Nov 14, 2018
@k8s-ci-robot k8s-ci-robot merged commit 4f96cd2 into kubernetes:master Nov 14, 2018
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. sig/architecture Categorizes an issue or PR as relevant to SIG Architecture. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.