Simplified virtual node name users need to define in the CRDs

[jqmichael]

Issue #, if available:
#14

Description of changes:
This change simplify virtual node name user needs to define in the CRDs.

Tests:
Follow the color example after naming simplification, and update the weights, everything works as expected.

Given CRD spec

apiVersion: appmesh.k8s.aws/v1alpha1
kind: VirtualService
metadata:
  name: colorteller.appmesh-demo.svc.cluster.local
  namespace: appmesh-demo
spec:
  meshName: color-mesh
  routes:
  - http:
      action:
        weightedTargets:
        - virtualNodeName: colorteller.appmesh-demo
          weight: 0
        - virtualNodeName: colorteller-blue
          weight: 3
        - virtualNodeName: colorteller-black.appmesh-demo
          weight: 3
      match:
        prefix: /
    name: color-route-appmesh-demo
  virtualRouter:
    name: color-router-appmesh-demo

apiVersion: appmesh.k8s.aws/v1alpha1
kind: VirtualNode
metadata:
  name: colorteller-black
  namespace: appmesh-demo

AppMesh backend representation:

$ aws appmesh list-virtual-nodes --mesh-name color-mesh
{
    "virtualNodes": [
        {
            "meshName": "color-mesh", 
            "arn": "arn:aws:appmesh:us-west-2:705114051414:mesh/color-mesh/virtualNode/colorteller-blue-appmesh-demo", 
            "virtualNodeName": "colorteller-blue-appmesh-demo"
        }, 
        {
            "meshName": "color-mesh", 
            "arn": "arn:aws:appmesh:us-west-2:705114051414:mesh/color-mesh/virtualNode/colorteller-black-appmesh-demo", 
            "virtualNodeName": "colorteller-black-appmesh-demo"
        }, 
        {
            "meshName": "color-mesh", 
            "arn": "arn:aws:appmesh:us-west-2:705114051414:mesh/color-mesh/virtualNode/colorteller-red-appmesh-demo", 
            "virtualNodeName": "colorteller-red-appmesh-demo"
        }, 
        {
            "meshName": "color-mesh", 
            "arn": "arn:aws:appmesh:us-west-2:705114051414:mesh/color-mesh/virtualNode/colorteller-appmesh-demo", 
            "virtualNodeName": "colorteller-appmesh-demo"
        }, 
        {
            "meshName": "color-mesh", 
            "arn": "arn:aws:appmesh:us-west-2:705114051414:mesh/color-mesh/virtualNode/colorgateway-appmesh-demo", 
            "virtualNodeName": "colorgateway-appmesh-demo"
        }
    ]
}

$ aws appmesh describe-route --mesh-name color-mesh \
    --virtual-router-name color-router-appmesh-demo --route-name color-route-appmesh-demo
{
    "route": {
        "status": {
            "status": "ACTIVE"
        }, 
        "meshName": "color-mesh", 
        "virtualRouterName": "color-router-appmesh-demo", 
        "routeName": "color-route-appmesh-demo", 
        "spec": {
            "httpRoute": {
                "action": {
                    "weightedTargets": [
                        {
                            "virtualNode": "colorteller-appmesh-demo", 
                            "weight": 0
                        }, 
                        {
                            "virtualNode": "colorteller-blue-appmesh-demo", 
                            "weight": 3
                        }, 
                        {
                            "virtualNode": "colorteller-black-appmesh-demo", 
                            "weight": 3
                        }
                    ]
                }, 
                "match": {
                    "prefix": "/"
                }
            }
        }
    }
}

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[jqmichael]

The docker base image needs to be updated to go 12.

[stefanprodan]

@jqmichael I think the routes and virtual routers should follow the same convention.

Instead of:

  routes:
  - name: color-route-appmesh-demo
  virtualRouter:
    name: color-router-appmesh-demo

could be:

  routes:
  - name: color-route # appmesh id: color-route-appmesh-demo
  virtualRouter:
    name: color-router # appmesh id: color-router-appmesh-demo

[stefanprodan]

I've tested this PR with Flagger fluxcd/flagger#118 and it works ok.

The Docker build fails with go 1.11, I've changed the base image to golang:1.12-stretch, here is my test image stefanprodan/app-mesh-controller:0.0.1-alpha.6

[nckturner]

Nice job. I do think we should do this for virtual routers and potentially virtual services, though each can be in a separate change.

[nckturner]

-defaultNamespace could also mean the namespace called default, I would say -virtualNodeNamespace to disambiguate.

[jqmichael]

I changed this to defaultVirtualNamespace to make it clear that this isn't a namespace that every virtual node should pick up. Let me know if this isn't clear.

[nckturner]

I don't think virtualNamespace is clear, what is a virtual namespace? defaultVirtualNodeNamespace?

[jqmichael]

Oops, that was typo. defaultVirtualNodeNamespace is what I meant.

[nckturner]

Nit: func ConstructAppMeshVNodeName(

[jqmichael]

I really prefer adding the CRD part just to make it clear to the consumer of this API that there're two virtual node name we are talking about, 1) App Mesh rss and 2) the name in the CRD.

Let me know if you have a strong preference not adding the CRD in the name.