-
Notifications
You must be signed in to change notification settings - Fork 40k
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
Specify Pod hostname by Annotation #20688
Conversation
Labelling this PR as size/L |
GCE e2e build/test failed for commit 7652cdbd96bec1a93f78518fc4d8b79c982196e7. |
PR needs rebase |
7652cdb
to
55e46ff
Compare
GCE e2e build/test failed for commit 55e46ff75921f1ea062f72189f8e253da7186a92. |
55e46ff
to
ff104a6
Compare
GCE e2e build/test failed for commit ff104a65646a632459c70a0200ef01f9203851c6. |
ff104a6
to
8b515bb
Compare
GCE e2e test build/test passed for commit 8b515bb514e22815aadcc9d0278845a8ad2574c8. |
8b515bb
to
c4ea9b9
Compare
GCE e2e test build/test passed for commit c4ea9b97d378a09857617f9d3a19972dcd75b651. |
c4ea9b9
to
567c75d
Compare
GCE e2e build/test failed for commit 567c75d0f0a412ccaab43365f6c866d9a83b9e23. |
567c75d
to
4f2a737
Compare
@@ -62,7 +64,8 @@ const ( | |||
// A subdomain added to the user specified domain for all services. | |||
serviceSubdomain = "svc" | |||
// A subdomain added to the user specified dmoain for all pods. | |||
podSubdomain = "pod" | |||
podSubdomain = "pod" | |||
podHostNamesAnnotation = "net.beta.kubernetes.io/podHostnames" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should use the symbol from the shared pkg, no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done.
GCE e2e build/test failed for commit 4f2a737e0c3f3310d974016760dba6b22278fdfb. |
@@ -507,6 +521,7 @@ func (dm *DockerManager) runContainer( | |||
Env: makeEnvList(opts.Envs), | |||
ExposedPorts: exposedPorts, | |||
Hostname: containerHostname, | |||
Domainname: containerSubdomain, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
did you figure out why it wasn't working when you showed me before?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we figured it out :)
That might be an interesting DNSPolicy option, but it's not one we have On Thu, Mar 3, 2016 at 1:40 PM, Derek Carr notifications@github.com wrote:
|
https://github.com/kubernetes/kubernetes/blob/master/cluster/addons/dns/kube2sky/kube2sky.go#L92 https://github.com/kubernetes/kubernetes/blob/master/cluster/addons/dns/kube2sky/kube2sky.go#L230 Is the goal to get rid of the above (seems we watch pods already in dns)? |
Yes, we will get rid of that when we do what you guys have already done On Thu, Mar 3, 2016 at 2:23 PM, Derek Carr notifications@github.com wrote:
|
PR pushed for review. Updating DNS doc in separate PR. |
GCE e2e build/test failed for commit 3d58ae29771911b4ca7fc79ef0f1da31093a137d. Please reference the list of currently known flakes when examining this failure. If you request a re-test, you must reference the issue describing the flake. |
@@ -18,8 +18,8 @@ | |||
|
|||
.PHONY: all kube2sky container push clean test | |||
|
|||
TAG = 1.12 | |||
PREFIX = gcr.io/google_containers | |||
TAG = 3.03.16-1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Push it and update the tags so we can merge? Need to merge with @bprashanth's changes
@@ -1875,6 +1900,7 @@ func ValidatePodTemplateSpec(spec *api.PodTemplateSpec, fldPath *field.Path) fie | |||
allErrs := field.ErrorList{} | |||
allErrs = append(allErrs, ValidateLabels(spec.Labels, fldPath.Child("labels"))...) | |||
allErrs = append(allErrs, ValidateAnnotations(spec.Annotations, fldPath.Child("annotations"))...) | |||
allErrs = append(allErrs, ValidatePodSpecificAnnotations(spec.Annotations, fldPath.Child("annotations"))...) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
don't you need to call this from ValidatePod(), too ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had fixed this already. Can you take a look at the push I did 10 seconds
ago ?
On Fri, Mar 4, 2016 at 12:19 PM, Tim Hockin notifications@github.com
wrote:
In pkg/api/validation/validation.go
#20688 (comment)
:@@ -1875,6 +1900,7 @@ func ValidatePodTemplateSpec(spec *api.PodTemplateSpec, fldPath *field.Path) fie
allErrs := field.ErrorList{}
allErrs = append(allErrs, ValidateLabels(spec.Labels, fldPath.Child("labels"))...)
allErrs = append(allErrs, ValidateAnnotations(spec.Annotations, fldPath.Child("annotations"))...)
- allErrs = append(allErrs, ValidatePodSpecificAnnotations(spec.Annotations, fldPath.Child("annotations"))...)
don't you need to call this from ValidatePod(), too ?
—
Reply to this email directly or view it on GitHub
https://github.com/kubernetes/kubernetes/pull/20688/files#r55083329.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LG
LGTM Just nits about the image push and tags |
GCE e2e build/test passed for commit 93673fe4d0e3ae8fda386b492d976c965e98b65d. |
@ArtfulCoder PR needs rebase |
The hostname is a DNS A record, if the subdomain maps to a service name in the same namespace
rebased, fixed image tag. |
LGTM |
GCE e2e build/test passed for commit a3c00aa. |
Specify Pod hostname by Annotation
Documentation: #22564