Skip to content

Commit

Permalink
Merge pull request #547 from Shopify/add-loadbalancer-condition
Browse files Browse the repository at this point in the history
Verify status.loadBalancer when deploying a service of type LoadBalancer
  • Loading branch information
dirceu authored Sep 10, 2019
2 parents 3fbc68b + 4eaae24 commit 3d5cf59
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 0 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
## next

*Enhancements*
- Make sure that we only declare a Service of type LoadBalancer as deployed after its IP address is published. [#547](https://github.com/Shopify/kubernetes-deploy/pull/547)

*Other*

- We've added a new Krane cli. This code is in alpha. We are providing
Expand Down
11 changes: 11 additions & 0 deletions lib/kubernetes-deploy/kubernetes_resource/service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ def sync(cache)
def status
if !exists?
"Not found"
elsif requires_publishing? && !published?
"LoadBalancer IP address is not provisioned yet"
elsif !requires_endpoints?
"Doesn't require any endpoints"
elsif selects_some_pods?
Expand All @@ -30,6 +32,7 @@ def status

def deploy_succeeded?
return false unless exists?
return published? if requires_publishing?
return exists? unless requires_endpoints?
# We can't use endpoints if we want the service to be able to fail fast when the pods are down
exposes_zero_replica_workload? || selects_some_pods?
Expand Down Expand Up @@ -89,5 +92,13 @@ def related_replica_count
def external_name_svc?
@definition["spec"]["type"] == "ExternalName"
end

def requires_publishing?
@definition["spec"]["type"] == "LoadBalancer"
end

def published?
@instance_data.dig('status', 'loadBalancer', 'ingress').present?
end
end
end
9 changes: 9 additions & 0 deletions test/fixtures/for_unit_tests/service_test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -200,3 +200,12 @@ spec:
containers:
- name: app
image: busybox
---
apiVersion: v1
kind: Service
metadata:
name: standard-lb
spec:
type: LoadBalancer
selector:
type: standard
33 changes: 33 additions & 0 deletions test/unit/kubernetes-deploy/kubernetes_resource/service_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,39 @@ def test_service_finds_deployment_with_different_pod_and_workload_labels
assert_equal("Doesn't require any endpoints", svc.status)
end

def test_ensures_populated_status_for_load_balancers
svc_def = service_fixture('standard-lb')
svc = build_service(svc_def)

stub_kind_get("Service", items: [svc_def])
stub_kind_get("Deployment", items: deployment_fixtures)
stub_kind_get("Pod", items: pod_fixtures)
stub_kind_get("StatefulSet", items: [])
svc.sync(build_resource_cache)

assert_includes(svc.to_yaml, 'type: LoadBalancer')
assert(svc.exists?)
refute(svc.deploy_succeeded?)
assert_equal("LoadBalancer IP address is not provisioned yet", svc.status)

svc_def = svc_def.deep_merge('status' => {
'loadBalancer' => {
'ingress' => [{
'ip' => '146.148.47.155',
}],
},
})
stub_kind_get("Service", items: [svc_def])
stub_kind_get("Deployment", items: deployment_fixtures)
stub_kind_get("Pod", items: pod_fixtures)
stub_kind_get("StatefulSet", items: [])
svc.sync(build_resource_cache)

assert(svc.exists?)
assert(svc.deploy_succeeded?)
assert_equal("Selects at least 1 pod", svc.status)
end

private

def build_service(definition)
Expand Down

0 comments on commit 3d5cf59

Please sign in to comment.