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

introduce the "registry" addon #1583

Merged
merged 1 commit into from
Jun 16, 2017
Merged

Conversation

bacongobbler
Copy link
Contributor

@bacongobbler bacongobbler commented Jun 13, 2017

This addon installs a docker registry into kube-system and exposes it on port 80 through the service.

A separate commit was also added to add 10.0.0.0/24 to the --insecure-registry flag by default so that the kubelets can communicate with the registry addon. I tested this functionality via

$ kubectl -n kube-system get svc registry -o jsonpath="{.spec.clusterIP}"
10.0.0.240
$ minikube ssh
$ docker pull busybox
$ docker tag busybox 10.0.0.240/busybox
$ docker push 10.0.0.240/busybox

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jun 13, 2017
@minikube-bot
Copy link
Collaborator

Can one of the admins verify this patch?

@r2d4
Copy link
Contributor

r2d4 commented Jun 13, 2017

@minikube-bot ok to test

@bacongobbler
Copy link
Contributor Author

I just realized that the registry replication controller contains some deployment concepts like the updateStrategy. Lemme fix that real quick

@bacongobbler
Copy link
Contributor Author

k, fixed

@codecov-io
Copy link

codecov-io commented Jun 13, 2017

Codecov Report

Merging #1583 into master will increase coverage by 0.07%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1583      +/-   ##
==========================================
+ Coverage    38.7%   38.78%   +0.07%     
==========================================
  Files          51       51              
  Lines        2604     2604              
==========================================
+ Hits         1008     1010       +2     
+ Misses       1418     1417       -1     
+ Partials      178      177       -1
Impacted Files Coverage Δ
cmd/minikube/cmd/start.go 17.05% <100%> (ø) ⬆️
pkg/minikube/kubeconfig/config.go 53.33% <0%> (+1.9%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7b9310d...5e1db75. Read the comment docs.

@bacongobbler
Copy link
Contributor Author

Seems like the kvm tests were flakes/unrelated issues.

@r2d4
Copy link
Contributor

r2d4 commented Jun 14, 2017

Yeah don't worry about those - we're working on the machines that run the linux and windows integration tests.

Copy link
Contributor

@r2d4 r2d4 left a comment

Choose a reason for hiding this comment

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

Sorry the review took a few days - a few comments

spec:
replicas: 1
selector:
kubernetes.io/minikube-addons: registry
Copy link
Contributor

Choose a reason for hiding this comment

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

These are ran by the addon-manager so they need the appropriate labels for it
Something like:
https://github.com/kubernetes/minikube/blob/master/deploy/addons/kube-dns/kube-dns-controller.yaml#L24

Copy link
Contributor Author

Choose a reason for hiding this comment

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

awesome. I'll update the docs on adding plugins.

@@ -142,6 +142,18 @@ var Addons = map[string]*Addon{
"ingress-svc.yaml",
"0640"),
}, false, "ingress"),
"registry": NewAddon([]*MemoryAsset{
Copy link
Contributor

Choose a reason for hiding this comment

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

The addons feature has a dependency on the minikube config feature, so these need an additional entry in the list here

https://github.com/kubernetes/minikube/blob/master/cmd/minikube/cmd/config/config.go#L45-L174

Then you can enable/disable by minikube addons enable registry

@bacongobbler bacongobbler force-pushed the registry-addon branch 2 times, most recently from 8f1f5a1 to c5dce93 Compare June 15, 2017 17:34
@bacongobbler
Copy link
Contributor Author

Comments addressed. Just a note that I have not been able to locally build minikube over here, so I have not been able to manually test this feature. Apologies for that!

@r2d4
Copy link
Contributor

r2d4 commented Jun 15, 2017

@bacongobbler why you can't build minikube locally? Maybe its something we can fix or document better.

@bacongobbler
Copy link
Contributor Author

bacongobbler commented Jun 16, 2017

Hmm, I'm not sure but things are working now. Things are building locally for me!

kind: Service
metadata:
labels:
kubernetes.io/minikube-addons: registry
Copy link
Contributor

Choose a reason for hiding this comment

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

The service also needs the addon-manager label addonmanager.kubernetes.io/mode: Reconcile. Unfortunately the addon-manager will ignore any resource without this label.

@r2d4
Copy link
Contributor

r2d4 commented Jun 16, 2017

@bacongobbler sorry one last label needed but otherwise LGTM!

@bacongobbler
Copy link
Contributor Author

Apologies. Thanks for catching that!

This addon installs a docker registry into the cluster.
Copy link
Contributor

@r2d4 r2d4 left a comment

Choose a reason for hiding this comment

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

Thanks! Once the workflow is finalized with draft/helm we should add some more documentation on how to use it

@r2d4 r2d4 merged commit c978526 into kubernetes:master Jun 16, 2017
@bacongobbler bacongobbler deleted the registry-addon branch June 16, 2017 17:58
@georgecrawford
Copy link

Is there documentation on how to use this yet?

@r2d4
Copy link
Contributor

r2d4 commented Jun 30, 2017

Not yet @georgecrawford but here's a rough sketch

# enable the registry addon
$ minikube addons enable registry

$ minikube start

# use the minikube docker daemon from the host
$ eval $(minikube docker-env)

# get the ip of the registry endpoint
$ kubectl -n kube-system get svc registry -o jsonpath="{.spec.clusterIP}"
10.0.0.240

# build and push to insecure registry
$ docker build -t 10.0.0.240/busybox .
$ docker push 10.0.0.240/busybox

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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants