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

Add custom pod conversion functions from pre-v1.1 mirror pods #16080

Merged
merged 1 commit into from
Oct 27, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
62 changes: 62 additions & 0 deletions pkg/api/v1/backward_compatibility_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,68 @@ func TestCompatibility_v1_PodSecurityContext(t *testing.T) {
"spec.hostPID",
},
},
{
name: "reseting defaults for pre-v1.1 mirror pods",
input: `
{
"kind":"Pod",
"apiVersion":"v1",
"metadata":{
"name":"my-pod-name",
"namespace":"my-pod-namespace",
"annotations": {
"kubernetes.io/config.mirror": "mirror"
}
},
"spec": {
"containers":[{
"name":"a",
"image":"my-container-image",
"resources": {
"limits": {
"cpu": "100m"
}
}
}]
}
}
`,
absentKeys: []string{
"spec.terminationGracePeriodSeconds",
"spec.containers[0].resources.requests",
},
},
{
name: "preserving defaults for v1.1+ mirror pods",
input: `
{
"kind":"Pod",
"apiVersion":"v1",
"metadata":{
"name":"my-pod-name",
"namespace":"my-pod-namespace",
"annotations": {
"kubernetes.io/config.mirror": "cbe924f710c7e26f7693d6a341bcfad0"
}
},
"spec": {
"containers":[{
"name":"a",
"image":"my-container-image",
"resources": {
"limits": {
"cpu": "100m"
}
}
}]
}
}
`,
expectedKeys: map[string]string{
"spec.terminationGracePeriodSeconds": "30",
"spec.containers[0].resources.requests": "map[cpu:100m]",
},
},
}

validator := func(obj runtime.Object) fielderrors.ValidationErrorList {
Expand Down
33 changes: 33 additions & 0 deletions pkg/api/v1/conversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,22 @@ import (
"k8s.io/kubernetes/pkg/conversion"
)

const (
// Annotation key used to identify mirror pods.
mirrorAnnotationKey = "kubernetes.io/config.mirror"
Copy link
Member

Choose a reason for hiding this comment

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

I realize this is too late now, but, for the future, this should have been something like kubelet.kubernetes.io/mirror-config. Prefix before the slash should be at least kubelet-specific, and suffix after the slash should use dashes rather than dots.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ack.


// Value used to identify mirror pods from pre-v1.1 kubelet.
mirrorAnnotationValue_1_0 = "mirror"
)

func addConversionFuncs() {
// Add non-generated conversion functions
err := api.Scheme.AddConversionFuncs(
convert_api_Pod_To_v1_Pod,
convert_api_PodSpec_To_v1_PodSpec,
convert_api_ReplicationControllerSpec_To_v1_ReplicationControllerSpec,
convert_api_ServiceSpec_To_v1_ServiceSpec,
convert_v1_Pod_To_api_Pod,
convert_v1_PodSpec_To_api_PodSpec,
convert_v1_ReplicationControllerSpec_To_api_ReplicationControllerSpec,
convert_v1_ServiceSpec_To_api_ServiceSpec,
Expand Down Expand Up @@ -386,9 +396,32 @@ func convert_v1_PodSpec_To_api_PodSpec(in *PodSpec, out *api.PodSpec, s conversi
} else {
out.ImagePullSecrets = nil
}

return nil
}

func convert_api_Pod_To_v1_Pod(in *api.Pod, out *Pod, s conversion.Scope) error {
if err := autoconvert_api_Pod_To_v1_Pod(in, out, s); err != nil {
return err
}
// We need to reset certain fields for mirror pods from pre-v1.1 kubelet
// (#15960).
// TODO: Remove this code after we drop support for v1.0 kubelets.
if value, ok := in.Annotations[mirrorAnnotationKey]; ok && value == mirrorAnnotationValue_1_0 {
// Reset the TerminationGracePeriodSeconds.
out.Spec.TerminationGracePeriodSeconds = nil
// Reset the resource requests.
for i := range out.Spec.Containers {
out.Spec.Containers[i].Resources.Requests = nil
}
}
return nil
}

func convert_v1_Pod_To_api_Pod(in *Pod, out *api.Pod, s conversion.Scope) error {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lavalamp, I don't think we need to muck with the conversion on this end since we will always reset the fields when converting to the versioned pod. Thoughts?

Copy link
Member

Choose a reason for hiding this comment

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

I agree.

On Thu, Oct 22, 2015 at 12:34 PM, Yu-Ju Hong notifications@github.com
wrote:

In pkg/api/v1/conversion.go
#16080 (comment)
:

  • }
  • // We need to reset certain fields for mirror pods from pre-v1.1 kubelet
  • // (Mirror pods in a delete create loop in version skewed cluster (1.1 master, 1.0 node) #15960).
  • // TODO: Remove this code after we drop support v1.0 kubelets.
  • if value, ok := in.Annotations[mirrorAnnotationKey]; ok && value == mirrorAnnotationValue {
  •   // Reset the TerminationGracePeriodSeconds.
    
  •   out.Spec.TerminationGracePeriodSeconds = nil
    
  •   // Reset the resource requests.
    
  •   for i := range out.Spec.Containers {
    
  •       out.Spec.Containers[i].Resources.Requests = nil
    
  •   }
    
  • }
  • return nil
    +}

+func convert_v1_Pod_To_api_Pod(in *Pod, out *api.Pod, s conversion.Scope) error {

@lavalamp https://github.com/lavalamp, I don't think we need to muck
with the conversion on this end since we will always reset the fields when
converting to the versioned pod. Thoughts?


Reply to this email directly or view it on GitHub
https://github.com/kubernetes/kubernetes/pull/16080/files#r42795288.

return autoconvert_v1_Pod_To_api_Pod(in, out, s)
}

func convert_api_ServiceSpec_To_v1_ServiceSpec(in *api.ServiceSpec, out *ServiceSpec, s conversion.Scope) error {
if err := autoconvert_api_ServiceSpec_To_v1_ServiceSpec(in, out, s); err != nil {
return err
Expand Down
8 changes: 0 additions & 8 deletions pkg/api/v1/conversion_generated.go
Original file line number Diff line number Diff line change
Expand Up @@ -1887,10 +1887,6 @@ func autoconvert_api_Pod_To_v1_Pod(in *api.Pod, out *Pod, s conversion.Scope) er
return nil
}

func convert_api_Pod_To_v1_Pod(in *api.Pod, out *Pod, s conversion.Scope) error {
return autoconvert_api_Pod_To_v1_Pod(in, out, s)
}

func autoconvert_api_PodAttachOptions_To_v1_PodAttachOptions(in *api.PodAttachOptions, out *PodAttachOptions, s conversion.Scope) error {
if defaulting, found := s.DefaultingInterface(reflect.TypeOf(*in)); found {
defaulting.(func(*api.PodAttachOptions))(in)
Expand Down Expand Up @@ -4912,10 +4908,6 @@ func autoconvert_v1_Pod_To_api_Pod(in *Pod, out *api.Pod, s conversion.Scope) er
return nil
}

func convert_v1_Pod_To_api_Pod(in *Pod, out *api.Pod, s conversion.Scope) error {
return autoconvert_v1_Pod_To_api_Pod(in, out, s)
}

func autoconvert_v1_PodAttachOptions_To_api_PodAttachOptions(in *PodAttachOptions, out *api.PodAttachOptions, s conversion.Scope) error {
if defaulting, found := s.DefaultingInterface(reflect.TypeOf(*in)); found {
defaulting.(func(*PodAttachOptions))(in)
Expand Down