fixup default --insecure-registry CIDR address #1603
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
|
Can one of the admins verify this patch? |
|
@minikube-bot ok to test |
|
See #1583 for when this was introduced |
|
It also appears that there are a few more bugs like the pods not being in the service's pod selector... More fixes to come in this PR! |
The default CIDR should be 10.0.0.0/24 rather than 10.0.0.1. Accidental off-by-one error. :)
bacongobbler
force-pushed
the
fixup-registry-addon
branch
from
5f8ace7
to
a3c5bf3
Compare
|
k, I'm going to manually test these changes before they're ready to merge to make sure I didn't mess anything up this time around. |
|
Okay, this now works as intended. From one pod in the cluster, pushing an image into the registry using the registry service's virtual IP address: Then, with a deployment pointing at the service IP address as the image name: All good to go :) |
|
@minikube-bot OK to test |
cmd/minikube/cmd/start.go
Outdated
| @@ -293,7 +293,7 @@ func init() { | |||
| startCmd.Flags().StringArrayVar(&dockerOpt, "docker-opt", nil, "Specify arbitrary flags to pass to the Docker daemon. (format: key=value)") | |||
| startCmd.Flags().String(apiServerName, constants.APIServerName, "The apiserver name which is used in the generated certificate for localkube/kubernetes. This can be used if you want to make the apiserver available from outside the machine") | |||
| startCmd.Flags().String(dnsDomain, constants.ClusterDNSDomain, "The cluster dns domain name used in the kubernetes cluster") | |||
| startCmd.Flags().StringSliceVar(&insecureRegistry, "insecure-registry", []string{util.DefaultServiceClusterIP + "/24"}, "Insecure Docker registries to pass to the Docker daemon") | |||
| startCmd.Flags().StringSliceVar(&insecureRegistry, "insecure-registry", []string{constants.DefaultInsecureRegistry}, "Insecure Docker registries to pass to the Docker daemon") | |||
There was a problem hiding this comment.
I think you need to reference this as "pkgutil.DefaultInsecureRegistry" (or move the constant definition to pkg/minikube/constants/constants.go)
There was a problem hiding this comment.
Sorry! I fixed this locally but never pushed up the change. Thanks!
bacongobbler
force-pushed
the
fixup-registry-addon
branch
from
a3c5bf3
to
b684071
Compare
Codecov Report
@@ Coverage Diff @@
## master #1603 +/- ##
=======================================
Coverage 38.66% 38.66%
=======================================
Files 51 51
Lines 2607 2607
=======================================
Hits 1008 1008
Misses 1421 1421
Partials 178 178
Continue to review full report at Codecov.
|
| template: | ||
| metadata: | ||
| labels: | ||
| addonmanager.kubernetes.io/mode: Reconcile |
There was a problem hiding this comment.
I think you need to keep this Reconcile label here in the template/metadata for the addon manager to pick it up.
There was a problem hiding this comment.
Is it required for the pod manifest as well? I guess replication controllers don't work the same as deployments. I'll add this back.
In a last-minute fix, I accidentally changed the pod labels all to the minikube add-on reconciliation mode label instead of the add-on name. This is causing the registry service to be unable to forward requests over to the pods due to mismatched label selectors.
bacongobbler
force-pushed
the
fixup-registry-addon
branch
from
b684071
to
cce611e
Compare
The default CIDR should be 10.0.0.0/24 rather than 10.0.0.1.
Accidental off-by-one error. :)