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

[aws-eks] restore_patch fails in KubernetesPatch #10801

Closed
2 tasks
melodyyangaws opened this issue Oct 9, 2020 · 4 comments
Closed
2 tasks

[aws-eks] restore_patch fails in KubernetesPatch #10801

melodyyangaws opened this issue Oct 9, 2020 · 4 comments
Assignees
Labels
@aws-cdk/aws-eks Related to Amazon Elastic Kubernetes Service feature-request A feature should be added or improved.

Comments

@melodyyangaws
Copy link

melodyyangaws commented Oct 9, 2020

Patch restore fails if switch from LoadBalancer service to ClusterIP. This is a known issue in Kubernetes: kubernetes/kubernetes#33766 . Is it possible to make the restore_patch an optional parameter?

Use Case

The following code usually works, however, if anything goes wrong, CDK will try to revert the service back to ClusterIP and throws an error: spec.ports[0].nodePort: Invalid value: 30179: may not be used when type is 'ClusterIP' .

patch = eks.KubernetesPatch( self, 'port-forwarding',
            cluster=my_cluster,
            resource_name='service/argo-server',
            apply_patch={"spec": {"type": "LoadBalancer"}},
            restore_patch={"spec": {"type": "ClusterIP"}}, 
            resource_namespace='argo'
        )

If someone intentionally change the type from external to internal balancing, ie. apply_patch -> ClusterIP, restore_patch -> LoadBalancer, the CDK deployment will fail.

Proposed Solution

The workaround is to manually delete the service or uninstall the entire application. Maybe make the restore_patch optional, or add some validation checks in the aws_eks module.

Other

kubernetes/kubernetes#33766

  • 👋 I may be able to implement this feature request
  • ⚠️ This feature might incur a breaking change

This is a 🚀 Feature Request

@melodyyangaws melodyyangaws added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Oct 9, 2020
@SomayaB SomayaB changed the title [aws_eks] restore_patch fails in KubernetesPatch [eks] restore_patch fails in KubernetesPatch Oct 12, 2020
@github-actions github-actions bot added the @aws-cdk/aws-eks Related to Amazon Elastic Kubernetes Service label Oct 12, 2020
@iliapolo
Copy link
Contributor

@melodyyangaws Do you mean that the apply_patch goes through fine but if something else in the stack fails (causing a stack rollback) it will try to invoke the restore_patch - which will now fail because of the issue you mentioned?

Are you able to workaround this issue by using an restore_patch={}?

@iliapolo iliapolo added response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. needs-discussion This issue/PR requires more discussion with community. and removed needs-triage This issue or PR still needs to be triaged. labels Oct 13, 2020
@iliapolo iliapolo changed the title [eks] restore_patch fails in KubernetesPatch [aws-eks] restore_patch fails in KubernetesPatch Oct 13, 2020
@melodyyangaws
Copy link
Author

@iliapolo yes, it will fail when stack rollback to restore_patch={"spec": {"type": "ClusterIP"}}. The workaround restore_patch={} works! Thanks.

@iliapolo
Copy link
Contributor

@melodyyangaws Great!

In general, requiring a restore_patch is good practice, and I wouldn't want to sacrifice that experience for the sake of supporting this scenario. Especially since a reasonable workaround exists.

I'm going to close this out in the meantime and if we accumulate more use-cases that justify making restore_patch optional we will consider it then.

@iliapolo iliapolo removed needs-discussion This issue/PR requires more discussion with community. response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. labels Oct 14, 2020
@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-eks Related to Amazon Elastic Kubernetes Service feature-request A feature should be added or improved.
Projects
None yet
Development

No branches or pull requests

2 participants