Skip to content

Commit

Permalink
fix: upgrade with CRD changes
Browse files Browse the repository at this point in the history
Probably since K8s 1.13.x, `converter.ConvertToVersion(info.Object, groupVersioner)` which is the body of `asVersioned` doesn't return an error or an "unstructured" object, but `apiextensions/v1beta1.CustomResourceDefinition`.

The result was `helm upgrade` with any changes in CRD consistently failing.

This fixes that by adding an additional case of the conversion result being `v1beta1.CustomResourceDefinition`.

This is a backward-compatible change as it doesn't remove existing switch cases for older K8s versions.

Fixes helm#5853

Signed-off-by: Yusuke Kuoka <ykuoka@gmail.com>
  • Loading branch information
mumoshu authored and Matthew Fisher committed Jul 30, 2019
1 parent a8b13cc commit 0e7f3b6
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 11 deletions.
35 changes: 25 additions & 10 deletions pkg/kube/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -648,24 +648,33 @@ func createPatch(target *resource.Info, current runtime.Object) ([]byte, types.P
}

// Get a versioned object
versionedObject := asVersioned(target)
versionedObject, err := asVersioned(target)

// Unstructured objects, such as CRDs, may not have an not registered error
// returned from ConvertToVersion. Anything that's unstructured should
// use the jsonpatch.CreateMergePatch. Strategic Merge Patch is not supported
// on objects like CRDs.
_, isUnstructured := versionedObject.(runtime.Unstructured)

// On newer K8s versions, CRDs aren't unstructured but has this dedicated type
_, isCRD := versionedObject.(*apiextv1beta1.CustomResourceDefinition)

switch {
case runtime.IsNotRegisteredError(err), isUnstructured:
case runtime.IsNotRegisteredError(err), isUnstructured, isCRD:
// fall back to generic JSON merge patch
patch, err := jsonpatch.CreateMergePatch(oldData, newData)
return patch, types.MergePatchType, err
if err != nil {
return nil, types.MergePatchType, fmt.Errorf("failed to create merge patch: %v", err)
}
return patch, types.MergePatchType, nil
case err != nil:
return nil, types.StrategicMergePatchType, fmt.Errorf("failed to get versionedObject: %s", err)
default:
patch, err := strategicpatch.CreateTwoWayMergePatch(oldData, newData, versionedObject)
return patch, types.StrategicMergePatchType, err
if err != nil {
return nil, types.StrategicMergePatchType, fmt.Errorf("failed to create two-way merge patch: %v", err)
}
return patch, types.StrategicMergePatchType, nil
}
}

Expand Down Expand Up @@ -720,7 +729,7 @@ func updateResource(c *Client, target *resource.Info, currentObj runtime.Object,
return nil
}

versioned := asVersioned(target)
versioned := asVersionedOrUnstructured(target)
selector, ok := getSelectorFromObject(versioned)
if !ok {
return nil
Expand Down Expand Up @@ -936,7 +945,7 @@ func (c *Client) getSelectRelationPod(info *resource.Info, objPods map[string][]

c.Log("get relation pod of object: %s/%s/%s", info.Namespace, info.Mapping.GroupVersionKind.Kind, info.Name)

versioned := asVersioned(info)
versioned := asVersionedOrUnstructured(info)
selector, ok := getSelectorFromObject(versioned)
if !ok {
return objPods, nil
Expand Down Expand Up @@ -969,17 +978,23 @@ func isFoundPod(podItem []v1.Pod, pod v1.Pod) bool {
return false
}

func asVersioned(info *resource.Info) runtime.Object {
func asVersionedOrUnstructured(info *resource.Info) runtime.Object {
obj, _ := asVersioned(info)
return obj
}

func asVersioned(info *resource.Info) (runtime.Object, error) {
converter := runtime.ObjectConvertor(scheme.Scheme)
groupVersioner := runtime.GroupVersioner(schema.GroupVersions(scheme.Scheme.PrioritizedVersionsAllGroups()))
if info.Mapping != nil {
groupVersioner = info.Mapping.GroupVersionKind.GroupVersion()
}

if obj, err := converter.ConvertToVersion(info.Object, groupVersioner); err == nil {
return obj
obj, err := converter.ConvertToVersion(info.Object, groupVersioner)
if err != nil {
return info.Object, err
}
return info.Object
return obj, nil
}

func asInternal(info *resource.Info) (runtime.Object, error) {
Expand Down
2 changes: 1 addition & 1 deletion pkg/kube/wait.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ func (c *Client) waitForResources(timeout time.Duration, created Result) error {
pvc := []v1.PersistentVolumeClaim{}
deployments := []deployment{}
for _, v := range created {
switch value := asVersioned(v).(type) {
switch value := asVersionedOrUnstructured(v).(type) {
case *v1.ReplicationController:
list, err := getPods(kcs, value.Namespace, value.Spec.Selector)
if err != nil {
Expand Down

0 comments on commit 0e7f3b6

Please sign in to comment.