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.

Sign up for GitHub

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

Jump to bottom

Use PatchForUpdate in Diff. #604

Merged
merged 3 commits into from
Jun 20, 2019
Merged

Use PatchForUpdate in Diff. #604

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b20ba74
Use PatchForUpdate in Diff.
pgavlin Jun 20, 2019
ee2cea3
Update the CHANGELOG
pgavlin Jun 20, 2019
c42a42f
Fix an omission
pgavlin Jun 20, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
## 0.25.1 (Unreleased)

### Improvements

- Unify diff behavior between `Diff` and `Update`. This should result in better detection of state drift as well as behavior that is more consistent with respect to `kubectl`.

## 0.25.0 (June 19, 2019)

### Supported Kubernetes versions
Expand Down
2 changes: 1 addition & 1 deletion go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,14 @@ require (
github.com/google/gofuzz v1.0.0 // indirect
github.com/googleapis/gnostic v0.2.0
github.com/gophercloud/gophercloud v0.0.0-20190418141522-bb98932a7b3a // indirect
github.com/grpc/grpc-go v0.0.0-00010101000000-000000000000 // indirect
github.com/imdario/mergo v0.3.7 // indirect
github.com/jinzhu/copier v0.0.0-20180308034124-7e38e58719c3
github.com/json-iterator/go v1.1.6 // indirect
github.com/mitchellh/go-wordwrap v1.0.0
github.com/pkg/errors v0.8.1
github.com/pulumi/pulumi v0.17.15
github.com/stretchr/testify v1.2.2
github.com/yudai/gojsondiff v1.0.0
google.golang.org/grpc v1.20.1
k8s.io/api v0.0.0-20190313235455-40a48860b5ab
k8s.io/apimachinery v0.0.0-20190313205120-d7deff9243b1
Expand Down
6 changes: 6 additions & 0 deletions pkg/openapi/match.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,13 @@ func PropertiesChanged(oldObj, newObj map[string]interface{}, paths []string) ([
if err != nil {
return nil, err
}
return PatchPropertiesChanged(patch, paths)
}

// PatchPropertiesChanged scrapes the given patch object to see if any path specified in `paths` has
// been changed. Paths are specified as JSONPaths, e.g., `.spec.accessModes` refers to `{spec:
// {accessModes: {}}}`.
func PatchPropertiesChanged(patch map[string]interface{}, paths []string) ([]string, error) {
j := jsonpath.New("")
matches := []string{}
for _, path := range paths {
Expand Down
6 changes: 2 additions & 4 deletions pkg/provider/diff.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,7 @@ import (
"k8s.io/apimachinery/pkg/runtime/schema"
)

func forceNewProperties(
oldObj, newObj map[string]interface{}, gvk schema.GroupVersionKind,
) ([]string, error) {
func forceNewProperties(patch map[string]interface{}, gvk schema.GroupVersionKind) ([]string, error) {
props := metadataForceNewProperties(".metadata")
if group, groupExists := forceNew[gvk.Group]; groupExists {
if version, versionExists := group[gvk.Version]; versionExists {
Expand All @@ -31,7 +29,7 @@ func forceNewProperties(
}
}

return openapi.PropertiesChanged(oldObj, newObj, props)
return openapi.PatchPropertiesChanged(patch, props)
}

type groups map[string]versions
Expand Down
30 changes: 24 additions & 6 deletions pkg/provider/provider.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package provider

import (
"context"
"encoding/json"
"fmt"
"net/url"
"os"
Expand All @@ -37,7 +38,6 @@ import (
"github.com/pulumi/pulumi/pkg/util/contract"
"github.com/pulumi/pulumi/pkg/util/rpcutil/rpcerror"
pulumirpc "github.com/pulumi/pulumi/sdk/proto/go"
"github.com/yudai/gojsondiff"
"google.golang.org/grpc/codes"
"k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/api/meta"
Expand Down Expand Up @@ -218,7 +218,7 @@ func (k *kubeProvider) Configure(_ context.Context, req *pulumirpc.ConfigureRequ
// Compute config overrides.
overrides := &clientcmd.ConfigOverrides{
Context: clientapi.Context{
Cluster: vars["kubernetes:config:cluster"],
Cluster: vars["kubernetes:config:cluster"],
},
CurrentContext: vars["kubernetes:config:context"],
}
Expand Down Expand Up @@ -501,7 +501,7 @@ func (k *kubeProvider) Diff(
if err != nil {
return nil, err
}
oldInputs, _ := parseCheckpointObject(oldState)
oldInputs, oldLiveState := parseCheckpointObject(oldState)

// Get new resource inputs. The user is submitting these as an update.
newResInputs, err := plugin.UnmarshalProperties(req.GetNews(), plugin.MarshalOptions{
Expand Down Expand Up @@ -538,18 +538,35 @@ func (k *kubeProvider) Diff(
oldInputs.SetNamespace("")
newInputs.SetNamespace("")
}
if oldInputs.GroupVersionKind().Empty() {
Copy link
Member

Choose a reason for hiding this comment

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

Why is this necessary?

Copy link
Member Author

Choose a reason for hiding this comment

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

If we have an empty old input set, the old input set will not have a GVK. openapi.PatchForResourceUpdate looks at the old input set to determine the GVK it will use to lookup patch meta. This ensures that the old inputs have a GVK that can be used by openapi.PatchForResourceUpdate.

oldInputs.SetGroupVersionKind(gvk)
}

patch, _, err := openapi.PatchForResourceUpdate(
k.clientSet.DiscoveryClientCached, oldInputs, newInputs, oldLiveState)
if err != nil {
return nil, err
}
patchObj := map[string]interface{}{}
if err = json.Unmarshal(patch, &patchObj); err != nil {
return nil, err
}

// Decide whether to replace the resource.
replaces, err := forceNewProperties(oldInputs.Object, newInputs.Object, gvk)
replaces, err := forceNewProperties(patchObj, gvk)
if err != nil {
return nil, err
}

// Pack up PB, ship response back.
hasChanges := pulumirpc.DiffResponse_DIFF_NONE
diff := gojsondiff.New().CompareObjects(oldInputs.Object, newInputs.Object)
if len(diff.Deltas()) > 0 {

var changes []string
if len(patchObj) != 0 {
hasChanges = pulumirpc.DiffResponse_DIFF_SOME
for k := range patchObj {
changes = append(changes, k)
pgavlin marked this conversation as resolved.
Show resolved Hide resolved
}
}

// Delete before replacement if we are forced to replace the old object, and the new version of
Expand All @@ -571,6 +588,7 @@ func (k *kubeProvider) Diff(
Replaces: replaces,
Stables: []string{},
DeleteBeforeReplace: deleteBeforeReplace,
Diffs: changes,
}, nil
}

Expand Down