aws-controllers-k8s / community

AWS Controllers for Kubernetes (ACK) is a project enabling you to manage AWS services from Kubernetes
https://aws-controllers-k8s.github.io/community/
Apache License 2.0
2.39k stars 253 forks source link

elasticache-controller no longer supports Tags on any of its resources #915

Closed rbranche closed 3 years ago

rbranche commented 3 years ago

Describe the bug elasticache-controller no longer supports Tags on any of its resources.

Steps to reproduce

Expected outcome See #63. Tags should be supported on all resources that support them in AWS.

rbranche commented 3 years ago

I'm not entirely familiar with how the API generation works.

@echen-98 made this commit that looks like it sets the Tags to be ignored for generation:

lekonia:elasticache-controller.git{main} branche$ git show 1ad7aaf1
commit 1ad7aaf1790d46d40c0e9ee6f97df8a8e7527972
Author: Eric Chen <ercche@amazon.com>
Date:   Thu Jul 15 18:33:11 2021 -0700

    regenerate code, fix replication group delete templates, upgrade to runtime 0.6.0
...
diff --git a/apis/v1alpha1/generator.yaml b/apis/v1alpha1/generator.yaml
index ae39c9d..5aedf65 100644
--- a/apis/v1alpha1/generator.yaml
+++ b/apis/v1alpha1/generator.yaml
...
@@ -203,3 +210,9 @@ ignore:
     - ModifyReplicationGroupInput.SecurityGroupIds
     - ModifyReplicationGroupInput.EngineVersion
     - CreateReplicationGroupInput.GlobalReplicationGroupId
+    - CreateSnapshotInput.Tags
+    - CreateCacheParameterGroupInput.Tags
+    - CreateCacheSubnetGroupInput.Tags
+    - CreateReplicationGroupInput.Tags
+    - CreateUserInput.Tags
+    - CreateUserGroupInput.Tags
rbranche commented 3 years ago

@nmvk It looks like they were removed in this particular commit:

commit 0dff94088585ea0dbdb1cb6ca8a8f066c68a7475 (HEAD)
Author: Raghav Muddur <r@nmvk.com>
Date:   Fri Jun 4 03:41:23 2021 -0700

    Add LogDelivery to RG Coverage, UserGroup to Modify

    Add a CW log to RG coverage.
    UserGroup ModifyRG support

...
diff --git a/apis/v1alpha1/replication_group.go b/apis/v1alpha1/replication_group.go
index 5ad8057..8fc45ef 100644
--- a/apis/v1alpha1/replication_group.go
+++ b/apis/v1alpha1/replication_group.go
@@ -290,11 +290,6 @@ type ReplicationGroupSpec struct {
        // If you do not specify this parameter, ElastiCache automatically chooses an
        // appropriate time range.
        SnapshotWindow *string `json:"snapshotWindow,omitempty"`
-       // A list of tags to be added to this resource. Tags are comma-separated key,value
-       // pairs (e.g. Key=myKey, Value=myKeyValue. You can include multiple tags as
-       // shown following: Key=myKey, Value=myKeyValue Key=mySecondKey, Value=mySecondKeyValue.
-       // Tags on replication groups will be replicated to all nodes.
-       Tags []*Tag `json:"tags,omitempty"`
rbranche commented 3 years ago

@jaypipes: Since we discussed it this morning, I took a stab at doing the work here.

Initial changes here: aws-controllers-k8s/elasticache-controller#50

I had to update some of the test data due to the code-generator changes (automatically including defaults), but I was not able to figure out the final test failure which seems like a real failure:

lekonia:elasticache-controller{add-tags} branche$ go test ./...
?       github.com/aws-controllers-k8s/elasticache-controller/apis/v1alpha1 [no test files]
?       github.com/aws-controllers-k8s/elasticache-controller/cmd/controller    [no test files]
?       github.com/aws-controllers-k8s/elasticache-controller/mocks/aws-sdk-go/elasticache  [no test files]
?       github.com/aws-controllers-k8s/elasticache-controller/pkg/common    [no test files]
?       github.com/aws-controllers-k8s/elasticache-controller/pkg/resource  [no test files]
?       github.com/aws-controllers-k8s/elasticache-controller/pkg/resource/cache_parameter_group    [no test files]
?       github.com/aws-controllers-k8s/elasticache-controller/pkg/resource/cache_subnet_group   [no test files]
Starting test: Cluster mode disabled replication group
Running test scenario: ReadOne=DNE
Running test scenario: Create=InvalidInput
Running test scenario: Create
Running test scenario: ReadOne=NewlyCreated
Running test scenario: ReadOne=NoDiff
Running test scenario: Update=IncreaseReplicaCount
Running test scenario: Update=ScaleUp
Unexpected differences:
Path: {[Spec CacheNodeType]}, expected: 0xc000122500, actual: 0xc000030e50
Running test scenario: Update=UpgradeEngine
Running test scenario: DeleteImmediate
Running test scenario: DeleteInitiated
Running test scenario: Deleting
Test: Cluster mode disabled replication group completed.
Starting test: Cluster mode enabled replication group
Running test scenario: Create=CustomShardConfig
Running test scenario: Update=ShardConfigMismatch
Test: Cluster mode enabled replication group completed.
--- FAIL: TestDeclarativeTestSuite (0.02s)
    --- FAIL: TestDeclarativeTestSuite/Update=ScaleUp (0.00s)
        test_suite_runner.go:129:
                Error Trace:    test_suite_runner.go:129
                                            test_suite_runner.go:99
                Error:          Not equal:
                                expected: 0
                                actual  : 1
                Test:           TestDeclarativeTestSuite/Update=ScaleUp
FAIL
FAIL    github.com/aws-controllers-k8s/elasticache-controller/pkg/resource/replication_group    1.248s
ok      github.com/aws-controllers-k8s/elasticache-controller/pkg/resource/snapshot (cached)
?       github.com/aws-controllers-k8s/elasticache-controller/pkg/resource/user [no test files]
?       github.com/aws-controllers-k8s/elasticache-controller/pkg/resource/user_group   [no test files]
ok      github.com/aws-controllers-k8s/elasticache-controller/pkg/testutil  (cached)
?       github.com/aws-controllers-k8s/elasticache-controller/pkg/version   [no test files]
FAIL
1 ↵

lekonia:elasticache-controller{add-tags} branche$ cd pkg/resource/replication_group/
lekonia:replication_group{add-tags} branche$ go test -c -ldflags=-compressdwarf=false -gcflags=all='-N -l' .
lekonia:replication_group{add-tags} branche$ dlv exec replication_group.test -- -test.run TestDeclarativeTestSuite/Update=ScaleUp
Type 'help' for list of commands.
(dlv) b pkg/testutil/test_suite_runner.go:129
Breakpoint 1 (enabled) set at 0x262fdba for github.com/aws-controllers-k8s/elasticache-controller/pkg/testutil.(*TestSuiteRunner).assertExpectations() /Users/branche/.go/src/github.com/aws-controllers-k8s/elasticache-controller/pkg/testutil/test_suite_runner.go:129
(dlv) c
Starting test: Cluster mode disabled replication group
Running test scenario: ReadOne=DNE
Running test scenario: Create=InvalidInput
Running test scenario: Create
Running test scenario: ReadOne=NewlyCreated
Running test scenario: ReadOne=NoDiff
Running test scenario: Update=IncreaseReplicaCount
Running test scenario: Update=ScaleUp
> github.com/aws-controllers-k8s/elasticache-controller/pkg/testutil.(*TestSuiteRunner).assertExpectations() /Users/branche/.go/src/github.com/aws-controllers-k8s/elasticache-controller/pkg/testutil/test_suite_runner.go:129 (hits goroutine(67):1 total:1) (PC: 0x262fdba)
   124:     } else if expectation.LatestState != "" {
   125:         expectedLatest := runner.loadAWSResource(expectation.LatestState)
   126:         assert.NotNil(actual)
   127:
   128:         delta := runner.Delegate.ResourceDescriptor().Delta(expectedLatest, actual)
=> 129:         assert.Equal(0, len(delta.Differences))
   130:         if len(delta.Differences) > 0 {
   131:             fmt.Println("Unexpected differences:")
   132:             for _, difference := range delta.Differences {
   133:                 fmt.Printf("Path: %v, expected: %v, actual: %v\n", difference.Path, difference.A, difference.B)
   134:             }
(dlv) print actual.ko
*github.com/aws-controllers-k8s/elasticache-controller/apis/v1alpha1.ReplicationGroup {
    TypeMeta: k8s.io/apimachinery/pkg/apis/meta/v1.TypeMeta {
        Kind: "ReplicationGroup",
        APIVersion: "elasticache.services.k8s.aws/v1alpha1",},
    ObjectMeta: k8s.io/apimachinery/pkg/apis/meta/v1.ObjectMeta {
        Name: "",
        GenerateName: "",
        Namespace: "",
        SelfLink: "",
        UID: "",
        ResourceVersion: "",
        Generation: 0,
        CreationTimestamp: (*"k8s.io/apimachinery/pkg/apis/meta/v1.Time")(0xc0002a7488),
        DeletionTimestamp: *k8s.io/apimachinery/pkg/apis/meta/v1.Time nil,
        DeletionGracePeriodSeconds: *int64 nil,
        Labels: map[string]string nil,
        Annotations: map[string]string nil,
        OwnerReferences: []k8s.io/apimachinery/pkg/apis/meta/v1.OwnerReference len: 0, cap: 0, nil,
        Finalizers: []string len: 0, cap: 0, nil,
        ClusterName: "",
        ManagedFields: []k8s.io/apimachinery/pkg/apis/meta/v1.ManagedFieldsEntry len: 0, cap: 0, nil,},
    Spec: github.com/aws-controllers-k8s/elasticache-controller/apis/v1alpha1.ReplicationGroupSpec {
        AtRestEncryptionEnabled: *false,
        AuthToken: *github.com/aws-controllers-k8s/runtime/apis/core/v1alpha1.SecretKeyReference nil,
        AutomaticFailoverEnabled: *bool nil,
        CacheNodeType: *"cache.t3.micro",
        CacheParameterGroupName: *string nil,
        CacheSecurityGroupNames: []*string len: 0, cap: 0, nil,
        CacheSubnetGroupName: *string nil,
        Engine: *"redis",
        EngineVersion: *string nil,
        KMSKeyID: *string nil,
        LogDeliveryConfigurations: []*github.com/aws-controllers-k8s/elasticache-controller/apis/v1alpha1.LogDeliveryConfigurationRequest len: 0, cap: 0, nil,
        MultiAZEnabled: *bool nil,
        NodeGroupConfiguration: []*github.com/aws-controllers-k8s/elasticache-controller/apis/v1alpha1.NodeGroupConfiguration len: 0, cap: 0, nil,
        NotificationTopicARN: *string nil,
        NumNodeGroups: *1,
        Port: *int64 nil,
        PreferredCacheClusterAZs: []*string len: 0, cap: 0, nil,
        PreferredMaintenanceWindow: *string nil,
        PrimaryClusterID: *string nil,
        ReplicasPerNodeGroup: *1,
        ReplicationGroupDescription: *"cluster-mode disabled RG",
        ReplicationGroupID: *"rg-cmd",
        SecurityGroupIDs: []*string len: 0, cap: 0, nil,
        SnapshotARNs: []*string len: 0, cap: 0, nil,
        SnapshotName: *string nil,
        SnapshotRetentionLimit: *0,
        SnapshotWindow: *"09:00-10:00",
        Tags: []*github.com/aws-controllers-k8s/elasticache-controller/apis/v1alpha1.Tag len: 0, cap: 0, nil,
        TransitEncryptionEnabled: *false,
        UserGroupIDs: []*string len: 0, cap: 0, nil,},
    Status: github.com/aws-controllers-k8s/elasticache-controller/apis/v1alpha1.ReplicationGroupStatus {
        ACKResourceMetadata: *(*"github.com/aws-controllers-k8s/runtime/apis/core/v1alpha1.ResourceMetadata")(0xc0003ed490),
        Conditions: []*github.com/aws-controllers-k8s/runtime/apis/core/v1alpha1.Condition len: 1, cap: 1, [
            *(*"github.com/aws-controllers-k8s/runtime/apis/core/v1alpha1.Condition")(0xc00016ed40),
        ],
        AllowedScaleDownModifications: []*string len: 0, cap: 0, nil,
        AllowedScaleUpModifications: []*string len: 0, cap: 0, nil,
        AuthTokenEnabled: *bool nil,
        AuthTokenLastModifiedDate: *k8s.io/apimachinery/pkg/apis/meta/v1.Time nil,
        AutomaticFailover: *"disabled",
        ClusterEnabled: *false,
        ConfigurationEndpoint: *github.com/aws-controllers-k8s/elasticache-controller/apis/v1alpha1.Endpoint nil,
        Description: *"cluster-mode disabled RG",
        Events: []*github.com/aws-controllers-k8s/elasticache-controller/apis/v1alpha1.Event len: 1, cap: 1, [
            *(*"github.com/aws-controllers-k8s/elasticache-controller/apis/v1alpha1.Event")(0xc0000a1720),
        ],
        GlobalReplicationGroupInfo: *(*"github.com/aws-controllers-k8s/elasticache-controller/apis/v1alpha1.GlobalReplicationGroupInfo")(0xc0003ed4f0),
        LogDeliveryConfigurations: []*github.com/aws-controllers-k8s/elasticache-controller/apis/v1alpha1.LogDeliveryConfiguration len: 0, cap: 0, nil,
        MemberClusters: []*string len: 2, cap: 2, [
            *"rg-cmd-001",
            *"rg-cmd-002",
        ],
        MemberClustersOutpostARNs: []*string len: 0, cap: 0, nil,
        MultiAZ: *"disabled",
        NodeGroups: []*github.com/aws-controllers-k8s/elasticache-controller/apis/v1alpha1.NodeGroup len: 1, cap: 1, [
            *(*"github.com/aws-controllers-k8s/elasticache-controller/apis/v1alpha1.NodeGroup")(0xc00016ed80),
        ],
        PendingModifiedValues: *(*"github.com/aws-controllers-k8s/elasticache-controller/apis/v1alpha1.ReplicationGroupPendingModifiedValues")(0xc00016edc0),
        SnapshottingClusterID: *string nil,
        Status: *"modifying",},}
(dlv) print expectedLatest.ko
*github.com/aws-controllers-k8s/elasticache-controller/apis/v1alpha1.ReplicationGroup {
    TypeMeta: k8s.io/apimachinery/pkg/apis/meta/v1.TypeMeta {
        Kind: "ReplicationGroup",
        APIVersion: "elasticache.services.k8s.aws/v1alpha1",},
    ObjectMeta: k8s.io/apimachinery/pkg/apis/meta/v1.ObjectMeta {
        Name: "",
        GenerateName: "",
        Namespace: "",
        SelfLink: "",
        UID: "",
        ResourceVersion: "",
        Generation: 0,
        CreationTimestamp: (*"k8s.io/apimachinery/pkg/apis/meta/v1.Time")(0xc0002a7888),
        DeletionTimestamp: *k8s.io/apimachinery/pkg/apis/meta/v1.Time nil,
        DeletionGracePeriodSeconds: *int64 nil,
        Labels: map[string]string nil,
        Annotations: map[string]string nil,
        OwnerReferences: []k8s.io/apimachinery/pkg/apis/meta/v1.OwnerReference len: 0, cap: 0, nil,
        Finalizers: []string len: 0, cap: 0, nil,
        ClusterName: "",
        ManagedFields: []k8s.io/apimachinery/pkg/apis/meta/v1.ManagedFieldsEntry len: 0, cap: 0, nil,},
    Spec: github.com/aws-controllers-k8s/elasticache-controller/apis/v1alpha1.ReplicationGroupSpec {
        AtRestEncryptionEnabled: *false,
        AuthToken: *github.com/aws-controllers-k8s/runtime/apis/core/v1alpha1.SecretKeyReference nil,
        AutomaticFailoverEnabled: *bool nil,
        CacheNodeType: *"cache.t3.small",
        CacheParameterGroupName: *string nil,
        CacheSecurityGroupNames: []*string len: 0, cap: 0, nil,
        CacheSubnetGroupName: *string nil,
        Engine: *"redis",
        EngineVersion: *string nil,
        KMSKeyID: *string nil,
        LogDeliveryConfigurations: []*github.com/aws-controllers-k8s/elasticache-controller/apis/v1alpha1.LogDeliveryConfigurationRequest len: 0, cap: 0, nil,
        MultiAZEnabled: *bool nil,
        NodeGroupConfiguration: []*github.com/aws-controllers-k8s/elasticache-controller/apis/v1alpha1.NodeGroupConfiguration len: 0, cap: 0, nil,
        NotificationTopicARN: *string nil,
        NumNodeGroups: *1,
        Port: *int64 nil,
        PreferredCacheClusterAZs: []*string len: 0, cap: 0, nil,
        PreferredMaintenanceWindow: *string nil,
        PrimaryClusterID: *string nil,
        ReplicasPerNodeGroup: *1,
        ReplicationGroupDescription: *"cluster-mode disabled RG",
        ReplicationGroupID: *"rg-cmd",
        SecurityGroupIDs: []*string len: 0, cap: 0, nil,
        SnapshotARNs: []*string len: 0, cap: 0, nil,
        SnapshotName: *string nil,
        SnapshotRetentionLimit: *0,
        SnapshotWindow: *"09:00-10:00",
        Tags: []*github.com/aws-controllers-k8s/elasticache-controller/apis/v1alpha1.Tag len: 0, cap: 0, nil,
        TransitEncryptionEnabled: *false,
        UserGroupIDs: []*string len: 0, cap: 0, nil,},
    Status: github.com/aws-controllers-k8s/elasticache-controller/apis/v1alpha1.ReplicationGroupStatus {
        ACKResourceMetadata: *(*"github.com/aws-controllers-k8s/runtime/apis/core/v1alpha1.ResourceMetadata")(0xc0004124d0),
        Conditions: []*github.com/aws-controllers-k8s/runtime/apis/core/v1alpha1.Condition len: 1, cap: 4, [
            *(*"github.com/aws-controllers-k8s/runtime/apis/core/v1alpha1.Condition")(0xc00016f240),
        ],
        AllowedScaleDownModifications: []*string len: 0, cap: 0, nil,
        AllowedScaleUpModifications: []*string len: 0, cap: 0, nil,
        AuthTokenEnabled: *bool nil,
        AuthTokenLastModifiedDate: *k8s.io/apimachinery/pkg/apis/meta/v1.Time nil,
        AutomaticFailover: *"disabled",
        ClusterEnabled: *false,
        ConfigurationEndpoint: *github.com/aws-controllers-k8s/elasticache-controller/apis/v1alpha1.Endpoint nil,
        Description: *"cluster-mode disabled RG",
        Events: []*github.com/aws-controllers-k8s/elasticache-controller/apis/v1alpha1.Event len: 1, cap: 4, [
            *(*"github.com/aws-controllers-k8s/elasticache-controller/apis/v1alpha1.Event")(0xc0000a19e0),
        ],
        GlobalReplicationGroupInfo: *(*"github.com/aws-controllers-k8s/elasticache-controller/apis/v1alpha1.GlobalReplicationGroupInfo")(0xc000412530),
        LogDeliveryConfigurations: []*github.com/aws-controllers-k8s/elasticache-controller/apis/v1alpha1.LogDeliveryConfiguration len: 0, cap: 0, nil,
        MemberClusters: []*string len: 2, cap: 4, [
            *"rg-cmd-001",
            *"rg-cmd-002",
        ],
        MemberClustersOutpostARNs: []*string len: 0, cap: 0, nil,
        MultiAZ: *"disabled",
        NodeGroups: []*github.com/aws-controllers-k8s/elasticache-controller/apis/v1alpha1.NodeGroup len: 1, cap: 4, [
            *(*"github.com/aws-controllers-k8s/elasticache-controller/apis/v1alpha1.NodeGroup")(0xc00016f2c0),
        ],
        PendingModifiedValues: *(*"github.com/aws-controllers-k8s/elasticache-controller/apis/v1alpha1.ReplicationGroupPendingModifiedValues")(0xc00016f180),
        SnapshottingClusterID: *string nil,
        Status: *"modifying",},}

CacheNodeType: *"cache.t3.small", != CacheNodeType: *"cache.t3.micro",

I'm not sure if this is due to the template change that was needed or not:

lekonia:elasticache-controller{add-tags} branche$ git show ef5228131eb67456b00df7019987875e0a83552d
commit ef5228131eb67456b00df7019987875e0a83552d
Author: Ryan Branche <branche@adobe.com>
Date:   Mon Aug 30 15:51:46 2021 -0700

    Update template to work properly with new code-generator code.

diff --git a/templates/hooks/sdk_file_end.go.tpl b/templates/hooks/sdk_file_end.go.tpl
index 87a473e..d6c29cd 100644
--- a/templates/hooks/sdk_file_end.go.tpl
+++ b/templates/hooks/sdk_file_end.go.tpl
@@ -14,7 +14,7 @@ func (rm *resourceManager) set{{ $outputShape.ShapeName }}Output (
        // Merge in the information we read from the API call above to the copy of
        // the original Kubernetes object we passed to the function
        ko := r.ko.DeepCopy()
-{{ $createCode := GoCodeSetCreateOutput .CRD "resp" "ko" 1 true }}
+{{ $createCode := GoCodeSetCreateOutput .CRD "resp" "ko" 1 }}
 {{ $createCode }}
        rm.setStatusDefaults(ko)
 {{- if $hookCode := Hook .CRD "sdk_file_end_set_output_post_populate" }}

If somebody could help point me in the right direction or feel free to take the reins from here.

rbranche commented 3 years ago

This is a duplicate of #261.