Skip to content

Commit

Permalink
Enable PRC by default in GCP (#2277)
Browse files Browse the repository at this point in the history 
This rolls out PlanResourceChange to GCP.

Part of pulumi/pulumi-terraform-bridge#1785
  • Loading branch information
@VenelinMartinov
VenelinMartinov authored Aug 14, 2024
1 parent 41e7489 commit da30c6c
Show file tree
Hide file tree
Showing 3 changed files with 118 additions and 113 deletions.
157 changes: 116 additions & 41 deletions examples/examples_go_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import (
"math/rand"
"os"
"path/filepath"
"reflect"
"testing"

"github.com/pulumi/pulumi-gcp/sdk/v6/go/gcp/sql"
Expand Down Expand Up @@ -288,6 +287,38 @@ func TestLabelsCombinationsGo(t *testing.T) {
Labels: map[string]string{},
},
},
{
"empty value keeps old value",
labelsState{
DefaultLabels: map[string]string{},
Labels: map[string]string{"x": "s"},
},
labelsState{
DefaultLabels: map[string]string{},
Labels: map[string]string{"x": ""},
},
},
{
"empty initial value does not get overridden by default",
labelsState{
DefaultLabels: map[string]string{"x": "s"},
Labels: map[string]string{"x": ""},
},
labelsState{
DefaultLabels: map[string]string{"x": "s"},
Labels: map[string]string{"x": ""},
},
},
{
"label kept if default is empty",
labelsState{
Labels: map[string]string{"x": "s"},
},
labelsState{
DefaultLabels: map[string]string{"x": ""},
},

},
}

for _, tc := range testCases {
Expand Down Expand Up @@ -387,64 +418,115 @@ func (st labelsState) validateTransitionTo(t *testing.T, st2 labelsState) {
"state2": st2.serialize(t),
},
Quick: true,
SkipRefresh: true,
SkipRefresh: true,
DestroyOnCleanup: true,
})

integration.ProgramTest(t, &opts)
}

func (st labelsState) expectedLabelsPRC(prev labelsState) map[string]string {
// Note that the upstream provider actually takes a "" value for a label to mean "keep the previous value".
// This behaviour is exposed under PlanResourceChange
r := map[string]string{}
for k, v := range st.DefaultLabels {
if v != "" {
r[k] = v
} else {
if prev.DefaultLabels[k] != "" {
r[k] = prev.DefaultLabels[k]
} else if prev.Labels[k] != "" {
r[k] = prev.Labels[k]
}
type expectedLabelValueArg struct {
labels, defaults, prevLabels, prevDefaults valuePresentTuple
}

type valuePresentTuple struct {
value string
present bool
}

// This takes the combination of label, default, previous label and previous default
// and returns the expected new value of a label. "" means it should not be present in the result.
func expectedLabelValue(arg expectedLabelValueArg) string {
// The labels behaviour in GCP is non-trivial. The labels are a combination of DefaultLabels and Labels.
// The DefaultLabels are the labels that are set by the provider by default. The user can override these
// labels by setting them in the Labels field of the resource.
//
// Things get a bit more complicated with how the GCP TF provider treats empty ("") labels.
// Generally, "" is treated as "keep the previous value". However, if the value has always been "", then
// it is treated as "no value", which trumps any DefaultLabels.
// The test cases above try to illustrate this behaviour.
labels, defaults, prevLabels, prevDefaults := arg.labels, arg.defaults, arg.prevLabels, arg.prevDefaults
if !labels.present {
if !defaults.present {
return ""
}
}
for k, v := range st.Labels {
if v != "" {
r[k] = v
} else {
if prev.DefaultLabels[k] != "" {
r[k] = prev.DefaultLabels[k]
} else if prev.Labels[k] != "" {
r[k] = prev.Labels[k]
}

if defaults.value != "" {
return defaults.value
}

if prevLabels.present {
return prevLabels.value
}

return prevDefaults.value
}
return r

if labels.value != "" {
return labels.value
}

if prevLabels.present {
return prevLabels.value
}

if prevDefaults.present {
return prevDefaults.value
}

return ""
}

func (st labelsState) expectedLabels() map[string]string {
func (st labelsState) expectedLabels(prev labelsState) map[string]string {
// Note that the upstream provider actually takes a "" value for a label to mean "keep the previous value".
// This behaviour is exposed under PlanResourceChange
r := map[string]string{}
for k, v := range st.DefaultLabels {
r[k] = v

keys := map[string]struct{}{}
for k := range st.DefaultLabels {
keys[k] = struct{}{}
}
for k, v := range st.Labels {
r[k] = v
for k := range st.Labels {
keys[k] = struct{}{}
}

for k := range keys {
labelsVal, inLabels := st.Labels[k]
defaultsVal, inDefaults := st.DefaultLabels[k]
prevLabelsVal, inPrevLabels := prev.Labels[k]
prevDefaultsVal, inPrevDefaults := prev.DefaultLabels[k]

expectedVal := expectedLabelValue(
expectedLabelValueArg{
labels: valuePresentTuple{value: labelsVal, present: inLabels},
defaults: valuePresentTuple{value: defaultsVal, present: inDefaults},
prevLabels: valuePresentTuple{value: prevLabelsVal, present: inPrevLabels},
prevDefaults: valuePresentTuple{value: prevDefaultsVal, present: inPrevDefaults},
},
)

if expectedVal != "" {
r[k] = expectedVal
}
}

return r
}

func validateStateResult(phase int, st1, st2 labelsState) func(
t *testing.T,
stack integration.RuntimeValidationStackInfo,
) {
var st labelsState
var st, prev labelsState
switch phase {
case 1:
st = st1
prev = labelsState{}
default:
st = st2
prev = st1
}
expected := st.expectedLabels(prev)

return func(t *testing.T, stack integration.RuntimeValidationStackInfo) {
for k, v := range stack.Outputs {
Expand All @@ -453,15 +535,8 @@ func validateStateResult(phase int, st1, st2 labelsState) func(
err := json.Unmarshal([]byte(actualLabelsJSON), &actualLabels)
require.NoError(t, err)
t.Logf("phase: %d", phase)
t.Logf("state1: %v", st1.serialize(t))
prev := labelsState{}
if phase == 2 {
prev = st1
t.Logf("state2: %v", st2.serialize(t))
}
if !reflect.DeepEqual(actualLabels, st.expectedLabelsPRC(prev)) {
require.Equalf(t, st.expectedLabels(), actualLabels, "key=%s", k)
}
t.Logf("state: %v", st.serialize(t))
require.Equalf(t, expected, actualLabels, "key=%s", k)
t.Logf("key=%s labels are as expected: %v", k, actualLabelsJSON)
}
}
Expand Down
59 changes: 1 addition & 58 deletions provider/provider_yaml_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -386,64 +386,7 @@ func TestNodePoolGpuAcceleratorPanic(t *testing.T) {
"preview": true
},
"response": {
"properties": {
"id": "",
"initialNodeCount": 1,
"name": "gpu-node-pool-c90bfc0",
"nodeConfig": {
"advancedMachineFeatures": null,
"bootDiskKmsKey": "",
"containerdConfig": null,
"confidentialNodes": "04da6b54-80e4-46f7-96ec-b56ff0331ba9",
"diskSizeGb": 50,
"diskType": "04da6b54-80e4-46f7-96ec-b56ff0331ba9",
"effectiveTaints": "04da6b54-80e4-46f7-96ec-b56ff0331ba9",
"enableConfidentialStorage": false,
"ephemeralStorageConfig": null,
"ephemeralStorageLocalSsdConfig": null,
"fastSocket": null,
"gcfsConfig": null,
"guestAccelerators": [
{
"count": 1,
"gpuDriverInstallationConfig": "04da6b54-80e4-46f7-96ec-b56ff0331ba9",
"gpuPartitionSize": "",
"gpuSharingConfig": null,
"type": "nvidia-tesla-t4"
}
],
"gvnic": null,
"hostMaintenancePolicy": null,
"imageType": "",
"kubeletConfig": null,
"labels": {},
"linuxNodeConfig": null,
"localNvmeSsdBlockConfig": null,
"localSsdCount": "04da6b54-80e4-46f7-96ec-b56ff0331ba9",
"loggingVariant": "04da6b54-80e4-46f7-96ec-b56ff0331ba9",
"machineType": "n1-highmem-8",
"metadata": "04da6b54-80e4-46f7-96ec-b56ff0331ba9",
"minCpuPlatform": "04da6b54-80e4-46f7-96ec-b56ff0331ba9",
"nodeGroup": "",
"oauthScopes": [
"https://www.googleapis.com/auth/cloud-platform"
],
"preemptible": false,
"reservationAffinity": null,
"resourceLabels": {},
"resourceManagerTags": {},
"sandboxConfig": null,
"serviceAccount": "04da6b54-80e4-46f7-96ec-b56ff0331ba9",
"secondaryBootDisks": [],
"shieldedInstanceConfig": "04da6b54-80e4-46f7-96ec-b56ff0331ba9",
"soleTenantConfig": null,
"spot": false,
"tags": [],
"taints": [],
"workloadMetadataConfig": "04da6b54-80e4-46f7-96ec-b56ff0331ba9"
},
"project": "pulumi-development"
}
"properties": "*"
},
"metadata": {
"kind": "resource",
Expand Down
15 changes: 1 addition & 14 deletions provider/resources.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -461,20 +461,7 @@ func Provider() tfbridge.ProviderInfo {
p := pf.MuxShimWithDisjointgPF(
context.Background(),
shimv2.NewProvider(gcpProvider.Provider(),
shimv2.WithDiffStrategy(shimv2.PlanState),
shimv2.WithPlanResourceChange(func(s string) bool {
switch s {
case "google_datastream_connection_profile",
"google_firestore_backup_schedule",
"google_cloud_run_service",
"google_cloud_run_domain_mapping",
"google_privileged_access_manager_entitlement",
"google_firestore_field":
return true
default:
return false
}
}),
shimv2.WithPlanResourceChange(func(_ string) bool { return true }),
),
gcpPFProvider.New(version.Version)) // this probably should be TF version but it does not seem to matter

Expand Down

0 comments on commit da30c6c

Please sign in to comment.