Merge pull request #75151 from jennybuckley/fix-conversion-bug

Fix apply conversion bug
This commit is contained in:
Kubernetes Prow Robot 2019-03-09 15:15:26 -08:00 committed by GitHub
commit 92b66c5c02
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 129 additions and 11 deletions

View File

@ -72,7 +72,7 @@ func NewCRDFieldManager(objectConverter runtime.ObjectConvertor, objectDefaulter
groupVersion: gv,
hubVersion: hub,
updater: merge.Updater{
Converter: internal.NewVersionConverter(internal.DeducedTypeConverter{}, objectConverter, hub),
Converter: internal.NewCRDVersionConverter(internal.DeducedTypeConverter{}, objectConverter, hub),
},
}
}

View File

@ -17,8 +17,6 @@ limitations under the License.
package internal
import (
"fmt"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
"sigs.k8s.io/structured-merge-diff/fieldpath"
@ -31,7 +29,7 @@ import (
type versionConverter struct {
typeConverter TypeConverter
objectConvertor runtime.ObjectConvertor
hubVersion schema.GroupVersion
hubGetter func(from schema.GroupVersion) schema.GroupVersion
}
var _ merge.Converter = &versionConverter{}
@ -41,7 +39,23 @@ func NewVersionConverter(t TypeConverter, o runtime.ObjectConvertor, h schema.Gr
return &versionConverter{
typeConverter: t,
objectConvertor: o,
hubVersion: h,
hubGetter: func(from schema.GroupVersion) schema.GroupVersion {
return schema.GroupVersion{
Group: from.Group,
Version: h.Version,
}
},
}
}
// NewCRDVersionConverter builds a VersionConverter for CRDs from a TypeConverter and an ObjectConvertor.
func NewCRDVersionConverter(t TypeConverter, o runtime.ObjectConvertor, h schema.GroupVersion) merge.Converter {
return &versionConverter{
typeConverter: t,
objectConvertor: o,
hubGetter: func(from schema.GroupVersion) schema.GroupVersion {
return h
},
}
}
@ -60,22 +74,21 @@ func (v *versionConverter) Convert(object typed.TypedValue, version fieldpath.AP
}
// If attempting to convert to the same version as we already have, just return it.
if objectToConvert.GetObjectKind().GroupVersionKind().GroupVersion() == groupVersion {
fromVersion := objectToConvert.GetObjectKind().GroupVersionKind().GroupVersion()
if fromVersion == groupVersion {
return object, nil
}
// Convert to internal
internalObject, err := v.objectConvertor.ConvertToVersion(objectToConvert, v.hubVersion)
internalObject, err := v.objectConvertor.ConvertToVersion(objectToConvert, v.hubGetter(fromVersion))
if err != nil {
return object, fmt.Errorf("failed to convert object (%v to %v): %v",
objectToConvert.GetObjectKind().GroupVersionKind(), v.hubVersion, err)
return object, err
}
// Convert the object into the target version
convertedObject, err := v.objectConvertor.ConvertToVersion(internalObject, groupVersion)
if err != nil {
return object, fmt.Errorf("failed to convert object (%v to %v): %v",
internalObject.GetObjectKind().GroupVersionKind(), groupVersion, err)
return object, err
}
// Convert the object back to a smd typed value and return it.

View File

@ -15,6 +15,7 @@ go_test(
"//pkg/master:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/api/meta:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/runtime/schema:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/types:go_default_library",
"//staging/src/k8s.io/apiserver/pkg/features:go_default_library",

View File

@ -24,6 +24,7 @@ import (
"k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/types"
genericfeatures "k8s.io/apiserver/pkg/features"
@ -438,3 +439,106 @@ func TestApplyRequiresFieldManager(t *testing.T) {
t.Fatalf("Apply failed to create with fieldManager: %v", err)
}
}
// TestApplyRemoveContainerPort removes a container port from a deployment
func TestApplyRemoveContainerPort(t *testing.T) {
defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, genericfeatures.ServerSideApply, true)()
_, client, closeFn := setup(t)
defer closeFn()
obj := []byte(`{
"apiVersion": "extensions/v1beta1",
"kind": "Deployment",
"metadata": {
"name": "deployment",
"labels": {"app": "nginx"}
},
"spec": {
"replicas": 3,
"selector": {
"matchLabels": {
"app": "nginx"
}
},
"template": {
"metadata": {
"labels": {
"app": "nginx"
}
},
"spec": {
"containers": [{
"name": "nginx",
"image": "nginx:latest",
"ports": [{
"containerPort": 80,
"protocol": "TCP"
}]
}]
}
}
}
}`)
_, err := client.CoreV1().RESTClient().Patch(types.ApplyPatchType).
AbsPath("/apis/extensions/v1beta1").
Namespace("default").
Resource("deployments").
Name("deployment").
Param("fieldManager", "apply_test").
Body(obj).Do().Get()
if err != nil {
t.Fatalf("Failed to create object using Apply patch: %v", err)
}
obj = []byte(`{
"apiVersion": "apps/v1",
"kind": "Deployment",
"metadata": {
"name": "deployment",
"labels": {"app": "nginx"}
},
"spec": {
"replicas": 3,
"selector": {
"matchLabels": {
"app": "nginx"
}
},
"template": {
"metadata": {
"labels": {
"app": "nginx"
}
},
"spec": {
"containers": [{
"name": "nginx",
"image": "nginx:latest"
}]
}
}
}
}`)
_, err = client.CoreV1().RESTClient().Patch(types.ApplyPatchType).
AbsPath("/apis/apps/v1").
Namespace("default").
Resource("deployments").
Name("deployment").
Param("fieldManager", "apply_test").
Body(obj).Do().Get()
if err != nil {
t.Fatalf("Failed to remove container port using Apply patch: %v", err)
}
deployment, err := client.AppsV1().Deployments("default").Get("deployment", metav1.GetOptions{})
if err != nil {
t.Fatalf("Failed to retrieve object: %v", err)
}
if len(deployment.Spec.Template.Spec.Containers[0].Ports) > 0 {
t.Fatalf("Expected no container ports but got: %v", deployment.Spec.Template.Spec.Containers[0].Ports)
}
}