-
Notifications
You must be signed in to change notification settings - Fork 112
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
Add load balancer service monitor resource and datasource #256
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good progress :)
edge_gateway = "my-edge-gw" | ||
|
||
name = "not-managed" | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about expanding example to not only show how to define this datasource, but also how to use it in some other resource?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can, but not yet. The next resource later on (vcd_lb_server_pool) will be able to consume it. Nothing else can use it now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, would be great to have that when possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Almost there, some final asks from my side.
edge_gateway = "my-edge-gw" | ||
|
||
name = "not-managed" | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, would be great to have that when possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job so far!
First pass. Some changes requested.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job, and thanks for the tune-ups! LGTM from my side (assuming no further changes to the user-facing interfaces).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM for now :)
Ref #223
Note Because load balancers depend on edge gateway having none other operations this PR will adopt locking functionality introduced in #255 before merge once it is in master.This is the first PR to start supporting edge gateway load balancers.
It includes resource and datasource
vcd_lb_service_monitor
. All actions should be possible includingimport
.