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.

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

(AppMesh): How to create two virtual service/node with bidirectional backend without circular dependencies #17322

Closed
FranzBusch opened this issue Nov 4, 2021 · 9 comments · Fixed by #18265
Assignees
Labels
@aws-cdk/aws-appmesh Related to AWS App Mesh bug This issue is a bug. effort/small Small work item – less than a day of effort needs-reproduction This issue needs reproduction. p1

Comments

@FranzBusch
Copy link

What is the problem?

I am currently setting up a CDK stack using ECS and AppMesh to create a micro service environment. In this micro service environment, every service should be able to talk to every other service. What I am trying to do is create a VirtualNode & Service per micro service and then call addBackend for each of these services and add all other services. However, this creates a cyclic dependencies for all services.

I haven't found a way yet to create this bidirectional backend setup without creating cyclic dependencies inside the CDK.

Reproduction Steps

        // Inside 1 stack
        const node1 = new appmesh.VirtualNode(stack1, 'node1', {
            virtualNodeName: 'Node1',
            mesh: props.mesh,
            listeners: [appmesh.VirtualNodeListener.grpc({
                port: 8080,
                healthCheck: appmesh.HealthCheck.grpc({
                    healthyThreshold: 2,
                    interval: cdk.Duration.seconds(5),
                    timeout: cdk.Duration.seconds(2),
                    unhealthyThreshold: 3,
                }),
            })],
            accessLog: appmesh.AccessLog.fromFilePath('/dev/stdout'),
        });

        const service1 = new appmesh.VirtualService(stack2, 'Service1', {
            virtualServiceName:  'Service1',
            virtualServiceProvider: appmesh.VirtualServiceProvider.virtualNode(node1),
        });

       // Inside stack 2
        const node2 = new appmesh.VirtualNode(this, 'node2', {
            virtualNodeName: 'Node2',
            mesh: props.mesh,
            listeners: [appmesh.VirtualNodeListener.grpc({
                port: 8080,
                healthCheck: appmesh.HealthCheck.grpc({
                    healthyThreshold: 2,
                    interval: cdk.Duration.seconds(5),
                    timeout: cdk.Duration.seconds(2),
                    unhealthyThreshold: 3,
                }),
            })],
            accessLog: appmesh.AccessLog.fromFilePath('/dev/stdout'),
        });

        const service2 = new appmesh.VirtualService(this, 'Service2', {
            virtualServiceName: 'Service2',
            virtualServiceProvider: appmesh.VirtualServiceProvider.virtualNode(node2),
        });

      // Added afterwards via Aspect
      node1.addBackend(service2);
      node2.addBackend(service1)

What did you expect to happen?

I expected there to be a way to create a bidirectional connection between backend services.

What actually happened?

It always creates a circular dependency and I can't see any escape hatch

CDK CLI Version

1.130.0

Framework Version

No response

Node.js Version

9.0.0

OS

MacOS BigSur

Language

Typescript

Language Version

No response

Other information

No response

@FranzBusch FranzBusch added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Nov 4, 2021
@github-actions github-actions bot added the @aws-cdk/aws-appmesh Related to AWS App Mesh label Nov 4, 2021
@Seiya6329
Copy link
Contributor

Hello @FranzBusch and thank you very much for submitting your issue. We will be looking into this and get back to you as soon as possible.

@ryparker ryparker added effort/small Small work item – less than a day of effort needs-reproduction This issue needs reproduction. and removed needs-triage This issue or PR still needs to be triaged. labels Nov 9, 2021
@Seiya6329
Copy link
Contributor

Hello @FranzBusch, sorry for the late response. We are still investigating this issue

@ashanpw
Copy link

ashanpw commented Nov 17, 2021

Thanks for the info. We are able to reproduce the issue and are now figuring out how to fix this or provide a workaround.

@mergify mergify bot closed this as completed in #18265 Jan 6, 2022
mergify bot pushed a commit that referenced this issue Jan 6, 2022
…e whose provider is that Node (#18265)

Addresses a circular dependency issue between Virtual Nodes and Virtual Services that works for Virtual Services created with a defined `virtualServiceName` and a randomly generated name. 

One such example of this problem was a case where a Virtual Node had a backend that is a Virtual Service whose provider was given as the same Virtual Node. This led to the Virtual Node being dependent on the creation of the Virtual Service, and the Virtual Service being dependent on the creation of the Virtual Node.

Fixes #17322

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@github-actions
Copy link

github-actions bot commented Jan 6, 2022

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

@PhilippBs
Copy link

PhilippBs commented Jan 9, 2022

I was also running into the same issue and sadly I think the merged code is not fixing this. The fix is when the backend of a virtual node is also the virtual service itself where it is provider. However, the issue above mentions that if we have two virtual nodes which have a virtual service respectively and we wanna create a bidirectional backend setup between them.

This is still running into circular dependencies for me and something that is super important to have in a micro service setup. The test code should look like this:

        // GIVEN
         const stack = new cdk.Stack();

         // WHEN
         const mesh = new appmesh.Mesh(stack, 'mesh', {
           meshName: 'test-mesh',
         });

         const node1 = new appmesh.VirtualNode(stack, 'test-node1', {
           mesh,
           serviceDiscovery: appmesh.ServiceDiscovery.dns('test1'),
         });

         const myVirtualService1 = new appmesh.VirtualService(stack, 'service-1', {
           virtualServiceProvider: appmesh.VirtualServiceProvider.virtualNode(node1),
           virtualServiceName: 'service1.domain.local',
         });

         const node2= new appmesh.VirtualNode(stack, 'test-node2', {
           mesh,
           serviceDiscovery: appmesh.ServiceDiscovery.dns('test2'),
         });

         const myVirtualService2 = new appmesh.VirtualService(stack, 'service-2', {
           virtualServiceProvider: appmesh.VirtualServiceProvider.virtualNode(node2),
           virtualServiceName: 'service2.domain.local',
         });

         node1.addBackend(appmesh.Backend.virtualService(myVirtualService2));
         node2.addBackend(appmesh.Backend.virtualService(myVirtualService1));

cc @ashanpw @Seiya6329

@Seiya6329 Seiya6329 reopened this Jan 10, 2022
@Seiya6329
Copy link
Contributor

@PhilippBs - Thanks for reporting the issue. We will further validate if the issue is still reproducible.

@FranzBusch - Please also let us know if you are experiencing the same issue.

@wplucinsky
Copy link
Contributor

@PhilippBs The latest version of @aws-cdk/aws-appmesh has not been published yet to NPM so the issue will persist until the CDK team publishes a new version. They do this every week around Wednesday. I will comment when it is updated. In the meantime you can use the link-all utility to pull in the latest changes.

You can tell the new vs old version by inspecting the CloudFormation template generated using cdk synth.

Old:

Backends:
  - VirtualService:
      VirtualServiceName:
        Fn::GetAtt:
          - service27C65CF7D
          - VirtualServiceName

New:

Backends:
  - VirtualService:
      VirtualServiceName: service1.domain.local

@wplucinsky
Copy link
Contributor

A new version of @aws-cdk/aws-appmesh has been published to NPM. Versions >= 1.139.0 contain the fix.

@github-actions
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

TikiTDO pushed a commit to TikiTDO/aws-cdk that referenced this issue Feb 21, 2022
…e whose provider is that Node (aws#18265)

Addresses a circular dependency issue between Virtual Nodes and Virtual Services that works for Virtual Services created with a defined `virtualServiceName` and a randomly generated name. 

One such example of this problem was a case where a Virtual Node had a backend that is a Virtual Service whose provider was given as the same Virtual Node. This led to the Virtual Node being dependent on the creation of the Virtual Service, and the Virtual Service being dependent on the creation of the Virtual Node.

Fixes aws#17322

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-appmesh Related to AWS App Mesh bug This issue is a bug. effort/small Small work item – less than a day of effort needs-reproduction This issue needs reproduction. p1
Projects
None yet
7 participants