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

move default image registry to quay #246

Merged

Conversation

andyxning
Copy link
Member

@andyxning andyxning commented Aug 28, 2017

This PR will move kube-state-metrics image registry from gcr.io to quay.io.

This is mainly because there are some times(#224 comment, #88 comment) the latest image can not be pushed to gcr.io timely. Users may be delayed to use the latest kube-state-metrics for this reason. A better way was to use quay.io as the docker registry as @brancz should be able to push the latest image when a new kube-state-metrics is released.

/cc @brancz @loburm @fabxc @piosz


This change is Reviewable

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Aug 28, 2017
Makefile Outdated
@@ -35,7 +35,7 @@ container: build
docker build -t ${REGISTRY}/kube-state-metrics:$(TAG) .

push: container
gcloud docker -- push ${REGISTRY}/kube-state-metrics:$(TAG)
Copy link
Member

Choose a reason for hiding this comment

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

As I don't use this make target, we can leave this for the google people as a help, as the image should still be published on gcr.

Copy link
Member Author

Choose a reason for hiding this comment

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

done.

README.md Outdated
@@ -27,7 +27,7 @@ Currently, `client-go` is in version `v4.0.0-beta.0`.

## Container Image

The latest container image can be found at `gcr.io/google_containers/kube-state-metrics:v0.5.0`.
The latest container image can be found at `quay.io/coreos/kube-state-metrics:v1.0.1`.
Copy link
Member

Choose a reason for hiding this comment

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

Let's enumerate both container image repositories here, but write a note, that the gcr image is only maintained on best effort as it requires external action from Google employees as no Google employee actively works on kube-state-metrics.

Copy link
Member Author

Choose a reason for hiding this comment

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

done. PTAL.

@brancz
Copy link
Member

brancz commented Aug 28, 2017

I'm fine with doing this, but we should still try to maintain the gcr repository, as people have expressed their interest in the gcr image before.

@piosz last time I opened a PR like this we closed it, as we said this process will be opened up to non google employees, what is the state on this, as we have trouble regarding this on every release?

@andyxning andyxning force-pushed the move_default_image_registry_to_quay branch from 6f503ea to 54e446b Compare August 28, 2017 13:57
@k8s-ci-robot k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Aug 28, 2017
@andyxning
Copy link
Member Author

andyxning commented Aug 28, 2017

@piosz @loburm This situation also have been encountered many times in the maintenance of heapster.

@piosz
Copy link
Member

piosz commented Aug 28, 2017

@kubernetes/sig-release-feature-requests could you please address the questions above?
@bgrant0607 is this ok to have official Kubernetes images in a different repo than gcr.io/google_containers ?

@piosz
Copy link
Member

piosz commented Aug 28, 2017

... and apologies for not doing great job here, but we only have a limited capacity. I agree that lack of access to the official Docker repository for non-googlers is painful.

@piosz
Copy link
Member

piosz commented Aug 28, 2017

/assign @piosz

@andyxning
Copy link
Member Author

Any updates?

@bgrant0607
Copy link
Member

@andyxning @brancz

In one of the previously cited PRs I see a comment "I pinged some Googlers". Who and how? Was the release team notified?

What is the release process for kube-state-metrics? At a glance, I didn't see this in the README and didn't see other obvious docs in the top level dir. Our repo template suggests RELEASE.md.

The general issue for solving this is:
kubernetes/release#281

How large is the image?

Also, how do users know which Kubernetes releases a given release of kube-state-metrics will work with?

@brancz
Copy link
Member

brancz commented Sep 5, 2017

Whenever we want to release a version of kube-state-metrics, I ping @piosz and @loburm as they are active members of sig-instrumentation and kube-state-metrics is owned by sig-instrumentation.

Previous attempts (that I could quickly find) around this include:

We picked up development from the previous maintainer and just continued as it was handled before: cut release, notify Googler to push image. I've previously been told that opening the release process is already in progress, so I for the time being decided to wait it out as the current approach wasn't giving us, as maintainers, too much pain.

How large is the image?

I'm not sure I understand the relevance, but my docker host is telling me 43.9MB.

Also, how do users know which Kubernetes releases a given release of kube-state-metrics will work with?

The README has a section on this, basically whatever client-go support matrix is.

I'll make sure we add a release process document, once we finish this discussion 🙂 .

@andyxning
Copy link
Member Author

@bgrant0607 One more thing. heapster has the same release image not found problem in

Was the release team notified?

@brancz I don't think we have a release team.

What is the release process for kube-state-metrics?

What is a reference implementation of a normalized release process?

@piosz
Copy link
Member

piosz commented Sep 5, 2017

@andyxning There is a dedicated @kubernetes/sig-release-members team.

@jdumars
Copy link
Member

jdumars commented Sep 5, 2017

@kubernetes/sig-release-members is not the release team, it's the SIG that sources many (but not all) release team members. https://github.com/orgs/kubernetes/teams/kubernetes-release-managers/members consists of the current release team.

@piosz
Copy link
Member

piosz commented Sep 5, 2017

@jdumars thanks for the info. Sorry for the confusion then.

@bgrant0607
Copy link
Member

@andyxning
Copy link
Member Author

andyxning commented Sep 7, 2017

@bgrant0607 Github releases can be used to ship binaries.

Seems github does not support image registry protocol, i.e., docker pull github.com/kubernetes/kube-state-metrics. :)

@bgrant0607
Copy link
Member

@andyxning Bits are bits. It's true that Github doesn't support pulling images via a registry API.

Anyway, this isn't part of the official Kubernetes release and doesn't seem to be used in https://github.com/kubernetes/kubernetes/tree/master/cluster/addons. Other repos (e.g., kompose) also push their release artifacts to random places. We haven't standardized that yet.

So long as no tests run by prow pull this image (we've had a lot of failures caused by other registries in the past), I don't have a strong opinion right now.

@piosz
Copy link
Member

piosz commented Sep 7, 2017

/lgtm
As we have the other Kubernetes projects not pushed to google_containers and this is not a part of cluster/addons, I'm fine with changing the default registry, so that we are not the bottleneck here. The only concern is that now CoreOS is the bottleneck but let's see how this will work. Google will also build all versions and push them to google_containers registry, so please keep pinging us when the new version is ready.

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 7, 2017
@brancz
Copy link
Member

brancz commented Sep 7, 2017

Yes I would suggest that we just document both image registries and users are free to choose which they want, and we'll continue as we do today.

@brancz brancz merged commit 7106e59 into kubernetes:master Sep 7, 2017
@andyxning andyxning deleted the move_default_image_registry_to_quay branch September 7, 2017 11:50
@andyxning
Copy link
Member Author

andyxning commented Sep 7, 2017

@piosz @bgrant0607 Thanks for your suggestions on this.

please keep pinging us when the new version is ready.

Absolutely. we need to work together to make kube-state-metrics friendly to users. :)

while1malloc0 pushed a commit to while1malloc0/kube-state-metrics that referenced this pull request Jul 2, 2018
…egistry_to_quay

move default image registry to quay
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants