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

[MM-49396] Add Name tag in ELB resources #826

Merged
merged 8 commits into from
Jan 11, 2023
Merged

[MM-49396] Add Name tag in ELB resources #826

merged 8 commits into from
Jan 11, 2023

Conversation

mirshahriar
Copy link
Contributor

Summary

Adding Name tag in ELB resources

Ticket Link

Resolves MM-49396

Release Note

None

@mm-cloud-bot mm-cloud-bot added the release-note-none Denotes a PR that doesn't merit a release note. label Jan 9, 2023
Copy link
Contributor

@fmartingr fmartingr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Only minor naming issues. Seems an elegant way to solve the problem of AWS providing two different packages so we can still operate over classic load balancers. But that poses a question. Should we migrate classic load balancers? If AWS is stopping providing support this could be a worse problem in the future if the APIs get deprecated. IIRC for now is safe to have CLBs on a VPC, but who knows what could happen. cc: @gabrieljackson @stylianosrigas

internal/tools/aws/elb_types.go Outdated Show resolved Hide resolved
internal/tools/aws/elb_types.go Outdated Show resolved Hide resolved
internal/tools/aws/elb.go Outdated Show resolved Hide resolved
mirshahriar and others added 2 commits January 9, 2023 22:21
Co-authored-by: Felipe Martin <812088+fmartingr@users.noreply.github.com>
@mirshahriar mirshahriar self-assigned this Jan 9, 2023
@mirshahriar mirshahriar added the 2: Dev Review Requires review by a developer label Jan 9, 2023
Copy link
Contributor

@gabrieljackson gabrieljackson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great. Just need to resolve a few more merge conflicts.

internal/tools/aws/elb.go Outdated Show resolved Hide resolved
@mirshahriar mirshahriar merged commit 2d4412b into master Jan 11, 2023
@mirshahriar mirshahriar deleted the MM-49396 branch January 11, 2023 05:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2: Dev Review Requires review by a developer release-note-none Denotes a PR that doesn't merit a release note.
Projects
None yet
5 participants