-
Notifications
You must be signed in to change notification settings - Fork 976
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
resource/kubernetes_service: Wait for LoadBalancer ingress #12
Conversation
Awesome! I've started to write similar PR, but yours much better. How do you run acceptance tests? I've tried to run it against GKE cluster, but it seems that it doesn't support the test when you are explicitly set LoadBalancerIP. |
Ha, good question - I do have a configuration of service w/ I never tried to set the |
Hm. I probably misunderstood something. I am looking at resource_kubernetes_service_test.go and it look like the spec for this test is
When I run this test against GKE cluster I can see something like this
|
You're absolutely right that the config we have in the acceptance test is creating service which doesn't actually work in GKE because it cannot assign the IP and this PR is exposing that fact by making the test fail. I'll rework the test + see if I can pull the failures from the events API and show the reason to the user. |
Failing test fixed, thanks again for noticing.
I will address the exposure of failure reasons in a separate PR as it doesn't affect just k8s service, but also Pod, RC and PVC. |
Awesome! Thank you! By the way - could you check if |
@e-max The deletion of LBs and associated resources is AFAIK asynchronous and I don't think we have access to the status of that operation through K8S API. I can tell it always gets deleted successfully, within ~20-30secs after deletion of the service. Below is relevant snippet from the event log generated by a
|
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.
a couple of minor questions - otherwise LGTM :)
return resource.NonRetryableError(err) | ||
} | ||
|
||
lbIngress := svc.Status.LoadBalancer.Ingress |
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.
do we want to check the status code before acting, or will the Get
method above handle returning an error for a non-ok status code?
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.
As discussed via Slack - in case the service suddenly doesn't exist it would get caught by the err check above, if the creation of LB failed then we'd time out after 5 mins. Exposing the reason of failed creation in the timeout error message is a job for a separate PR.
@@ -159,6 +202,11 @@ func resourceKubernetesServiceRead(d *schema.ResourceData, meta interface{}) err | |||
return err | |||
} | |||
|
|||
err = d.Set("load_balancer_ingress", flattenLoadBalancerIngress(svc.Status.LoadBalancer.Ingress)) |
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.
should we add a test to check this value?
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.
The K8S always returned empty struct consistently - i.e. svc.Status
was never nil
so I don't think so.
CronJob resource
This is to allow the user to easily access the exposed IP/hostname for the app, e.g.