Skip to content

Commit

Permalink
Controller gets stuck in a crash loop (#314)
Browse files Browse the repository at this point in the history 
* CloudMap config of VirtualNode Spec is made immutable

* additional test case for virtualnode validating webhook

* Allow DNS->CloudMap modification
  • Loading branch information
@achevuru
achevuru authored Jul 16, 2020
1 parent f5aecef commit 675cef2
Show file tree
Hide file tree
Showing 3 changed files with 176 additions and 4 deletions.
2 changes: 0 additions & 2 deletions controllers/appmesh/cloudmap_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,6 @@ func (r *cloudMapReconciler) reconcileVirtualNodeWithCloudMap(ctx context.Contex

func (r *cloudMapReconciler) cleanupCloudMapResources(ctx context.Context, vNode *appmesh.VirtualNode) error {
if k8s.HasFinalizer(vNode, k8s.FinalizerAWSCloudMapResources) {
// TODO: choose one of the approach in https://github.com/aws/aws-app-mesh-controller-for-k8s/issues/305#issuecomment-654374069
// to completely fix the issue
if vNode.Spec.ServiceDiscovery != nil && vNode.Spec.ServiceDiscovery.AWSCloudMap != nil {
if err := r.cloudMapResourceManager.Cleanup(ctx, vNode); err != nil {
return err
Expand Down
3 changes: 3 additions & 0 deletions webhooks/appmesh/virtualnode_validator.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,9 @@ func (v *virtualNodeValidator) enforceFieldsImmutability(vn *appmesh.VirtualNode
if !reflect.DeepEqual(vn.Spec.MeshRef, oldVN.Spec.MeshRef) {
changedImmutableFields = append(changedImmutableFields, "spec.meshRef")
}
if oldVN.Spec.ServiceDiscovery.AWSCloudMap != nil && !reflect.DeepEqual(vn.Spec.ServiceDiscovery.AWSCloudMap, oldVN.Spec.ServiceDiscovery.AWSCloudMap) {
changedImmutableFields = append(changedImmutableFields, "spec.serviceDiscovery.awsCloudMap")
}
if len(changedImmutableFields) != 0 {
return errors.Errorf("%s update may not change these fields: %s", "VirtualNode", strings.Join(changedImmutableFields, ","))
}
Expand Down
175 changes: 173 additions & 2 deletions webhooks/appmesh/virtualnode_validator_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,94 @@ func Test_virtualNodeValidator_enforceFieldsImmutability(t *testing.T) {
Name: "my-mesh",
UID: "408d3036-7dec-11ea-b156-0e30aabe1ca8",
},
ServiceDiscovery: &appmesh.ServiceDiscovery{
AWSCloudMap: &appmesh.AWSCloudMapServiceDiscovery{
NamespaceName: "cloudmap-ns",
ServiceName: "cloudmap-svc",
},
},
},
},
oldVN: &appmesh.VirtualNode{
ObjectMeta: metav1.ObjectMeta{
Namespace: "awesome-ns",
Name: "my-vn",
},
Spec: appmesh.VirtualNodeSpec{
AWSName: aws.String("my-vn_awesome-ns"),
MeshRef: &appmesh.MeshReference{
Name: "my-mesh",
UID: "408d3036-7dec-11ea-b156-0e30aabe1ca8",
},
ServiceDiscovery: &appmesh.ServiceDiscovery{
AWSCloudMap: &appmesh.AWSCloudMapServiceDiscovery{
NamespaceName: "cloudmap-ns",
ServiceName: "cloudmap-svc",
},
},
},
},
},
wantErr: nil,
},
{
name: "VirtualNode DNS Servicediscovery change is allowed",
args: args{
vn: &appmesh.VirtualNode{
ObjectMeta: metav1.ObjectMeta{
Namespace: "awesome-ns",
Name: "my-vn",
},
Spec: appmesh.VirtualNodeSpec{
AWSName: aws.String("my-vn_awesome-ns"),
MeshRef: &appmesh.MeshReference{
Name: "my-mesh",
UID: "408d3036-7dec-11ea-b156-0e30aabe1ca8",
},
ServiceDiscovery: &appmesh.ServiceDiscovery{
DNS: &appmesh.DNSServiceDiscovery{Hostname: "dns-new-hostname"},
},
},
},
oldVN: &appmesh.VirtualNode{
ObjectMeta: metav1.ObjectMeta{
Namespace: "awesome-ns",
Name: "my-vn",
},
Spec: appmesh.VirtualNodeSpec{
AWSName: aws.String("my-vn_awesome-ns"),
MeshRef: &appmesh.MeshReference{
Name: "my-mesh",
UID: "408d3036-7dec-11ea-b156-0e30aabe1ca8",
},
ServiceDiscovery: &appmesh.ServiceDiscovery{
DNS: &appmesh.DNSServiceDiscovery{Hostname: "dns-hostname"},
},
},
},
},
wantErr: nil,
},
{
name: "VirtualNode Servicediscovery mode change from DNS to CloudMap is allowed",
args: args{
vn: &appmesh.VirtualNode{
ObjectMeta: metav1.ObjectMeta{
Namespace: "awesome-ns",
Name: "my-vn",
},
Spec: appmesh.VirtualNodeSpec{
AWSName: aws.String("my-vn_awesome-ns"),
MeshRef: &appmesh.MeshReference{
Name: "my-mesh",
UID: "408d3036-7dec-11ea-b156-0e30aabe1ca8",
},
ServiceDiscovery: &appmesh.ServiceDiscovery{
AWSCloudMap: &appmesh.AWSCloudMapServiceDiscovery{
NamespaceName: "cloudmap-ns",
ServiceName: "cloudmap-svc",
},
},
},
},
oldVN: &appmesh.VirtualNode{
Expand All @@ -46,6 +134,9 @@ func Test_virtualNodeValidator_enforceFieldsImmutability(t *testing.T) {
Name: "my-mesh",
UID: "408d3036-7dec-11ea-b156-0e30aabe1ca8",
},
ServiceDiscovery: &appmesh.ServiceDiscovery{
DNS: &appmesh.DNSServiceDiscovery{Hostname: "dns-hostname"},
},
},
},
},
Expand All @@ -65,6 +156,12 @@ func Test_virtualNodeValidator_enforceFieldsImmutability(t *testing.T) {
Name: "my-mesh",
UID: "408d3036-7dec-11ea-b156-0e30aabe1ca8",
},
ServiceDiscovery: &appmesh.ServiceDiscovery{
AWSCloudMap: &appmesh.AWSCloudMapServiceDiscovery{
NamespaceName: "cloudmap-ns",
ServiceName: "cloudmap-svc",
},
},
},
},
oldVN: &appmesh.VirtualNode{
Expand All @@ -78,6 +175,12 @@ func Test_virtualNodeValidator_enforceFieldsImmutability(t *testing.T) {
Name: "my-mesh",
UID: "408d3036-7dec-11ea-b156-0e30aabe1ca8",
},
ServiceDiscovery: &appmesh.ServiceDiscovery{
AWSCloudMap: &appmesh.AWSCloudMapServiceDiscovery{
NamespaceName: "cloudmap-ns",
ServiceName: "cloudmap-svc",
},
},
},
},
},
Expand All @@ -97,6 +200,12 @@ func Test_virtualNodeValidator_enforceFieldsImmutability(t *testing.T) {
Name: "another-mesh",
UID: "408d3036-7dec-11ea-b156-0e30aabe1ca8",
},
ServiceDiscovery: &appmesh.ServiceDiscovery{
AWSCloudMap: &appmesh.AWSCloudMapServiceDiscovery{
NamespaceName: "cloudmap-ns",
ServiceName: "cloudmap-svc",
},
},
},
},
oldVN: &appmesh.VirtualNode{
Expand All @@ -110,13 +219,63 @@ func Test_virtualNodeValidator_enforceFieldsImmutability(t *testing.T) {
Name: "my-mesh",
UID: "408d3036-7dec-11ea-b156-0e30aabe1ca8",
},
ServiceDiscovery: &appmesh.ServiceDiscovery{
AWSCloudMap: &appmesh.AWSCloudMapServiceDiscovery{
NamespaceName: "cloudmap-ns",
ServiceName: "cloudmap-svc",
},
},
},
},
},
wantErr: errors.New("VirtualNode update may not change these fields: spec.meshRef"),
},
{
name: "VirtualNode fields awsName and meshRef changed",
name: "VirtualNode field awsCloudMap changed",
args: args{
vn: &appmesh.VirtualNode{
ObjectMeta: metav1.ObjectMeta{
Namespace: "awesome-ns",
Name: "my-vn",
},
Spec: appmesh.VirtualNodeSpec{
AWSName: aws.String("my-vn_awesome-ns"),
MeshRef: &appmesh.MeshReference{
Name: "my-mesh",
UID: "408d3036-7dec-11ea-b156-0e30aabe1ca8",
},
ServiceDiscovery: &appmesh.ServiceDiscovery{
AWSCloudMap: &appmesh.AWSCloudMapServiceDiscovery{
NamespaceName: "new-cloudmap-ns",
ServiceName: "new-cloudmap-svc",
},
},
},
},
oldVN: &appmesh.VirtualNode{
ObjectMeta: metav1.ObjectMeta{
Namespace: "awesome-ns",
Name: "my-vn",
},
Spec: appmesh.VirtualNodeSpec{
AWSName: aws.String("my-vn_awesome-ns"),
MeshRef: &appmesh.MeshReference{
Name: "my-mesh",
UID: "408d3036-7dec-11ea-b156-0e30aabe1ca8",
},
ServiceDiscovery: &appmesh.ServiceDiscovery{
AWSCloudMap: &appmesh.AWSCloudMapServiceDiscovery{
NamespaceName: "cloudmap-ns",
ServiceName: "cloudmap-svc",
},
},
},
},
},
wantErr: errors.New("VirtualNode update may not change these fields: spec.serviceDiscovery.awsCloudMap"),
},
{
name: "VirtualNode fields awsName, meshRef and awsCloudMap changed",
args: args{
vn: &appmesh.VirtualNode{
ObjectMeta: metav1.ObjectMeta{
Expand All @@ -129,6 +288,12 @@ func Test_virtualNodeValidator_enforceFieldsImmutability(t *testing.T) {
Name: "another-mesh",
UID: "408d3036-7dec-11ea-b156-0e30aabe1ca8",
},
ServiceDiscovery: &appmesh.ServiceDiscovery{
AWSCloudMap: &appmesh.AWSCloudMapServiceDiscovery{
NamespaceName: "new-cloudmap-ns",
ServiceName: "new-cloudmap-svc",
},
},
},
},
oldVN: &appmesh.VirtualNode{
Expand All @@ -142,10 +307,16 @@ func Test_virtualNodeValidator_enforceFieldsImmutability(t *testing.T) {
Name: "my-mesh",
UID: "408d3036-7dec-11ea-b156-0e30aabe1ca8",
},
ServiceDiscovery: &appmesh.ServiceDiscovery{
AWSCloudMap: &appmesh.AWSCloudMapServiceDiscovery{
NamespaceName: "cloudmap-ns",
ServiceName: "cloudmap-svc",
},
},
},
},
},
wantErr: errors.New("VirtualNode update may not change these fields: spec.awsName,spec.meshRef"),
wantErr: errors.New("VirtualNode update may not change these fields: spec.awsName,spec.meshRef,spec.serviceDiscovery.awsCloudMap"),
},
}
for _, tt := range tests {
Expand Down

0 comments on commit 675cef2

Please sign in to comment.