update PodSpec.Host to PodSpec.NodeName #8853

caesarxuchao · 2015-05-27T04:10:38Z

update PodSpec.Host to PodSpec.NodeName in /pkg/api/types.go and /pkg/api/v1/types.go

bgrant0607 · 2015-05-27T05:51:33Z

pkg/api/v1/types.go

 	// the the scheduler simply schedules this pod onto that host, assuming that it fits
 	// resource requirements.
-	Host string `json:"host,omitempty" description:"host requested for this pod"`
+	NodeName string `json:"nodeName,omitempty" description:"host requested for this pod"`


Please change the description tag, also: node requested for this pod.

bgrant0607 · 2015-05-27T05:56:49Z

Fails gofmt

bgrant0607 · 2015-05-27T05:57:26Z

Ref #7018

caesarxuchao · 2015-05-27T17:32:17Z

Updated. PTAL. Thanks.

bgrant0607 · 2015-05-27T23:00:35Z

Doesn't build. Regenerate pkg/api/deep_copy_generated.go

caesarxuchao · 2015-05-27T23:09:45Z

Yeah, I have the problem when I work on #6797 today. It's strange this problem didn't occur yesterday. I will get back to this PR after fixing #6797.

bgrant0607 · 2015-05-27T23:41:32Z

@wojtek-t We're having trouble with the generated conversions. Is it obvious what we're doing wrong?

caesarxuchao · 2015-05-28T00:03:01Z

Hi @wojtek-t, no big deals, I just want to know what's the best practice when one tried to update the api, e.g., update PodSpec.Host to PodSpec.Nodename.

here are the steps that I want to take, but these don't work:

change a field name (e.g. Host->NodeName) in api/types.go and api/v1/types.go
add functions in v1beta3/conversion.go to convert "Host" in v1beta3 to "NodeName" in internal version
run hack/update-generated-conversion.sh and hack/update-generated-deep-copies.sh.
This doesn't work because in step 3, the scripts complain that the v1beta3/conversion-generated.go and deep-copies_generated.go contain undeclared field name "Host"

So I have to manually update various -generated.go, and then these update- scripts are happy. However, at this point, I have already made all the changes manually, there is no work left for these update-* scripts. Please let me know if there is a better way. Thanks.

bgrant0607 · 2015-05-28T00:05:53Z

pkg/controller/controller_utils.go

@@ -19,6 +19,8 @@ package controller
 import (
 	"fmt"

+	"sync/atomic"


Why this change?

There is something wrong with the goimports. I will fix it. Thanks.

bgrant0607 · 2015-05-28T00:17:57Z

Building the generator should not build the previously generated code. That seems wrong.

caesarxuchao · 2015-05-28T00:32:34Z

In cmd/genconversion/conversion.go:

     "github.com/GoogleCloudPlatform/kubernetes/pkg/api"
     _ "github.com/GoogleCloudPlatform/kubernetes/pkg/api/v1"
     _ "github.com/GoogleCloudPlatform/kubernetes/pkg/api/v1beta1"
     _ "github.com/GoogleCloudPlatform/kubernetes/pkg/api/v1beta2"
     _ "github.com/GoogleCloudPlatform/kubernetes/pkg/api/v1beta3"

So to build the generator, it requires the conversion_generated.go files in these packages to be correct.

bgrant0607 · 2015-05-28T00:37:41Z

I understand that's the way it is. I was saying it shouldn't be that way. :-)

The generated code just needs to build, though, right? It doesn't need to work? If you commented out the lines it complained about, would the generator build and then regenerate the conversions?

caesarxuchao · 2015-05-28T00:56:02Z

Yeah, that should work. That seems to be the best option.

wojtek-t · 2015-05-28T07:07:58Z

@caesarxuchao - yes, unfortunately this is the easiest way to do it. Basically, what you can do is to remove the whole code from */conversions_generated.go files, and running:
hack/update-generated-conversions.sh will regenerate them.
I agree it's not convenient and shouldn't work like this but it's not trivial to fix it.

bgrant0607 · 2015-05-28T14:10:32Z

@wojtek-t Please add that to the instructions.
https://github.com/GoogleCloudPlatform/kubernetes/blob/master/docs/devel/api_changes.md#edit-version-conversions

wojtek-t · 2015-05-28T15:36:06Z

@bgrant0607 - I thought about it and I know how to fix the problem. Will fix the real issue instead of updating documentation :)

caesarxuchao · 2015-05-28T16:31:12Z

"remove the whole code from */conversions_generated.go" won't work, because some functions in conversion.go depend on code in conversions_generated.go

@wojtek-t, I'm looking forward to your fix : )

bgrant0607 · 2015-05-28T16:52:29Z

pkg/api/validation/validation.go

-	if newPod.Spec.Host != oldPod.Spec.Host {
-		allErrs = append(allErrs, errs.NewFieldInvalid("status.host", newPod.Spec.Host, "pod host cannot be changed directly"))
+	if newPod.Spec.NodeName != oldPod.Spec.NodeName {
+		allErrs = append(allErrs, errs.NewFieldInvalid("status.nodeName", newPod.Spec.NodeName, "pod host cannot be changed directly"))


s/host/node/ in error message

wojtek-t · 2015-05-28T16:53:56Z

@caesarxuchao unfortunately my fix doesn't work because what you wrote. Updated instructions are in #8921

bgrant0607 · 2015-05-28T16:54:41Z

Tests failing:

_output/local/go/src/github.com/GoogleCloudPlatform/kubernetes/pkg/api/deep_copy_generated.go:1377: out.Host undefined (type *PodSpec has no field or method Host)

caesarxuchao · 2015-05-28T17:38:39Z

@bgrant0607 Updated and rebased. Let's wait for the Shippable results. Thanks.

bgrant0607 · 2015-05-28T17:48:57Z

Shippable failed already:

 github.com/GoogleCloudPlatform/kubernetes/test/soak/serve_hostnames
test/soak/serve_hostnames/serve_hostnames.go:188: unknown "github.com/GoogleCloudPlatform/kubernetes/pkg/api".PodSpec field 'Host' in struct literal

caesarxuchao · 2015-05-28T18:11:50Z

There is something wrong in field.selector. I'm taking a look.

caesarxuchao · 2015-05-28T19:49:13Z

@bgrant0607 Shippable is green

bgrant0607 · 2015-05-28T20:22:42Z

Please squash the commits.

bgrant0607 · 2015-05-28T20:23:31Z

LGTM once squashed.

…/api/v1beta3/types.go

caesarxuchao · 2015-05-28T21:10:46Z

Squashed. Thanks.

update PodSpec.Host to PodSpec.NodeName

googlebot added the cla: yes label May 27, 2015

bgrant0607 added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label May 27, 2015

bgrant0607 reviewed May 27, 2015
View reviewed changes

bgrant0607 self-assigned this May 27, 2015

caesarxuchao force-pushed the HostToNodeName branch from 75a58ff to 7c53f20 Compare May 27, 2015 17:31

bgrant0607 reviewed May 28, 2015
View reviewed changes

caesarxuchao force-pushed the HostToNodeName branch 2 times, most recently from 241087d to 89cd3ab Compare May 28, 2015 17:35

bgrant0607 added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 28, 2015

update PodSpec.Host to PodSpec.NodeName in /pkg/api/types.go and /pkg…

9c11533

…/api/v1beta3/types.go

caesarxuchao force-pushed the HostToNodeName branch from 93e7baa to 9c11533 Compare May 28, 2015 21:10

thockin added a commit that referenced this pull request May 28, 2015

Merge pull request #8853 from caesarxuchao/HostToNodeName

081ab3a

update PodSpec.Host to PodSpec.NodeName

thockin merged commit 081ab3a into kubernetes:master May 28, 2015

caesarxuchao deleted the HostToNodeName branch May 12, 2020 23:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

update PodSpec.Host to PodSpec.NodeName #8853

update PodSpec.Host to PodSpec.NodeName #8853

caesarxuchao commented May 27, 2015

bgrant0607 May 27, 2015

bgrant0607 commented May 27, 2015

bgrant0607 commented May 27, 2015

caesarxuchao commented May 27, 2015

bgrant0607 commented May 27, 2015

caesarxuchao commented May 27, 2015

bgrant0607 commented May 27, 2015

caesarxuchao commented May 28, 2015

bgrant0607 May 28, 2015

caesarxuchao May 28, 2015

bgrant0607 commented May 28, 2015

caesarxuchao commented May 28, 2015

bgrant0607 commented May 28, 2015

caesarxuchao commented May 28, 2015

wojtek-t commented May 28, 2015

bgrant0607 commented May 28, 2015

wojtek-t commented May 28, 2015

caesarxuchao commented May 28, 2015

bgrant0607 May 28, 2015

wojtek-t commented May 28, 2015

bgrant0607 commented May 28, 2015

caesarxuchao commented May 28, 2015

bgrant0607 commented May 28, 2015

caesarxuchao commented May 28, 2015

caesarxuchao commented May 28, 2015

bgrant0607 commented May 28, 2015

bgrant0607 commented May 28, 2015

caesarxuchao commented May 28, 2015

update PodSpec.Host to PodSpec.NodeName #8853

update PodSpec.Host to PodSpec.NodeName #8853

Conversation

caesarxuchao commented May 27, 2015

bgrant0607 May 27, 2015

Choose a reason for hiding this comment

bgrant0607 commented May 27, 2015

bgrant0607 commented May 27, 2015

caesarxuchao commented May 27, 2015

bgrant0607 commented May 27, 2015

caesarxuchao commented May 27, 2015

bgrant0607 commented May 27, 2015

caesarxuchao commented May 28, 2015

bgrant0607 May 28, 2015

Choose a reason for hiding this comment

caesarxuchao May 28, 2015

Choose a reason for hiding this comment

bgrant0607 commented May 28, 2015

caesarxuchao commented May 28, 2015

bgrant0607 commented May 28, 2015

caesarxuchao commented May 28, 2015

wojtek-t commented May 28, 2015

bgrant0607 commented May 28, 2015

wojtek-t commented May 28, 2015

caesarxuchao commented May 28, 2015

bgrant0607 May 28, 2015

Choose a reason for hiding this comment

wojtek-t commented May 28, 2015

bgrant0607 commented May 28, 2015

caesarxuchao commented May 28, 2015

bgrant0607 commented May 28, 2015

caesarxuchao commented May 28, 2015

caesarxuchao commented May 28, 2015

bgrant0607 commented May 28, 2015

bgrant0607 commented May 28, 2015

caesarxuchao commented May 28, 2015