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

Specify Pod hostname by Annotation #20688

Merged
merged 1 commit into from
Mar 4, 2016

Conversation

ArtfulCoder
Copy link
Contributor

Documentation: #22564

@k8s-github-robot
Copy link

Labelling this PR as size/L

@k8s-github-robot k8s-github-robot added kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Feb 5, 2016
@k8s-bot
Copy link

k8s-bot commented Feb 5, 2016

GCE e2e build/test failed for commit 7652cdbd96bec1a93f78518fc4d8b79c982196e7.

@k8s-github-robot
Copy link

PR needs rebase

@k8s-github-robot k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 5, 2016
@k8s-bot
Copy link

k8s-bot commented Feb 5, 2016

GCE e2e build/test failed for commit 55e46ff75921f1ea062f72189f8e253da7186a92.

@k8s-github-robot k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 5, 2016
@k8s-bot
Copy link

k8s-bot commented Feb 5, 2016

GCE e2e build/test failed for commit ff104a65646a632459c70a0200ef01f9203851c6.

@k8s-bot
Copy link

k8s-bot commented Feb 9, 2016

GCE e2e test build/test passed for commit 8b515bb514e22815aadcc9d0278845a8ad2574c8.

@k8s-bot
Copy link

k8s-bot commented Feb 9, 2016

GCE e2e test build/test passed for commit c4ea9b97d378a09857617f9d3a19972dcd75b651.

@k8s-github-robot k8s-github-robot removed the kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API label Feb 9, 2016
@k8s-bot
Copy link

k8s-bot commented Feb 9, 2016

GCE e2e build/test failed for commit 567c75d0f0a412ccaab43365f6c866d9a83b9e23.

@@ -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"
Copy link
Member

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done.

@k8s-bot
Copy link

k8s-bot commented Feb 9, 2016

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,
Copy link
Member

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?

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 figured it out :)

@derekwaynecarr
Copy link
Member

@thockin - yeah, @csrwng and I realized that after I posted it ;-)

@thockin
Copy link
Member

thockin commented Mar 3, 2016

That might be an interesting DNSPolicy option, but it's not one we have
now.

On Thu, Mar 3, 2016 at 1:40 PM, Derek Carr notifications@github.com wrote:

@thockin https://github.com/thockin - yeah, @csrwng
https://github.com/csrwng and I realized that after I posted it ;-)


Reply to this email directly or view it on GitHub
#20688 (comment)
.

@thockin
Copy link
Member

thockin commented Mar 3, 2016

Yes, we will get rid of that when we do what you guys have already done
(use skyDNS backends)

On Thu, Mar 3, 2016 at 2:23 PM, Derek Carr notifications@github.com wrote:

@thockin https://github.com/thockin

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)?


Reply to this email directly or view it on GitHub
#20688 (comment)
.

@ArtfulCoder
Copy link
Contributor Author

PR pushed for review. Updating DNS doc in separate PR.

@k8s-bot
Copy link

k8s-bot commented Mar 4, 2016

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
Copy link
Member

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"))...)
Copy link
Member

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 ?

Copy link
Contributor Author

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.

Copy link
Member

Choose a reason for hiding this comment

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

LG

@thockin
Copy link
Member

thockin commented Mar 4, 2016

LGTM Just nits about the image push and tags

@k8s-bot
Copy link

k8s-bot commented Mar 4, 2016

GCE e2e build/test passed for commit 93673fe4d0e3ae8fda386b492d976c965e98b65d.

@k8s-github-robot
Copy link

@ArtfulCoder PR needs rebase

@k8s-github-robot k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 4, 2016
The hostname is a DNS A record, if the subdomain maps to a service name
in the same namespace
@k8s-github-robot k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 4, 2016
@ArtfulCoder
Copy link
Contributor Author

rebased, fixed image tag.

@thockin
Copy link
Member

thockin commented Mar 4, 2016

LGTM

@thockin thockin added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 4, 2016
@k8s-bot
Copy link

k8s-bot commented Mar 4, 2016

GCE e2e build/test passed for commit a3c00aa.

ArtfulCoder added a commit that referenced this pull request Mar 4, 2016
Specify Pod hostname by Annotation
@ArtfulCoder ArtfulCoder merged commit 9bfd70f into kubernetes:master Mar 4, 2016
k8s-github-robot pushed a commit that referenced this pull request Apr 29, 2016
Automatic merge from submit-queue

Promote Pod Hostname & Subdomain to fields (were annotations)

Deprecating the podHostName, subdomain and PodHostnames annotations and created corresponding new fields for them on PodSpec and Endpoints types.

Annotation doc: #22564
Annotation code: #20688
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm "Looks good to me", indicates that a PR is ready to be merged. 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.

None yet