Skip to content

Commit

Permalink
Merge pull request #3393 from puppetlabs/impl/detect-ambassador-servi…
Browse files Browse the repository at this point in the history
…ce-regression

Correctly detect the Ambassador service using selector and namespace
  • Loading branch information
kflynn authored May 7, 2021
2 parents 01e62cc + d12924a commit 1bbb412
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 8 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ Please see the [Envoy documentation](https://www.envoyproxy.io/docs/envoy/latest
### Emissary Ingress and Ambassador Edge Stack

- Change: `prune_unreachable_routes` now defaults to true, which should reduce Envoy memory requirements for installations with many `Host`s
- Bugfix: Fixed a regression in detecting the Ambassador Kubernetes service that could cause the wrong IP or hostname to be used in Ingress statuses

## [1.13.3] May 03, 2021
[1.13.3]: https://github.com/datawire/ambassador/compare/v1.13.2...v1.13.3
Expand Down
24 changes: 16 additions & 8 deletions python/ambassador/fetch/service.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,22 +56,30 @@ def __init__(self, manager: ResourceManager) -> None:
def kinds(self) -> FrozenSet[KubernetesGVK]:
return frozenset([KubernetesGVK('v1', 'Service')])

def _is_ambassador_service(self, service_labels: Dict[str, str], service_selector: Dict[str, str]) -> bool:
# self.logger.info(f"is_ambassador_service checking {service_labels} - {service_selector}")
def _is_ambassador_service(self, obj: KubernetesObject) -> bool:
selector = obj.spec.get('selector', {})
# self.logger.info(f"is_ambassador_service checking {obj.labels} - {selector}")

# Every Ambassador service must have the label 'app.kubernetes.io/component: ambassador-service'
if service_labels.get('app.kubernetes.io/component', "").lower() != 'ambassador-service':
if obj.labels.get('app.kubernetes.io/component', "").lower() != 'ambassador-service':
return False

# This service must be in the same namespace as the Ambassador deployment.
if obj.namespace != Config.ambassador_namespace:
return False

# Now that we have the Ambassador label, let's verify that this Ambassador service routes to this very
# Ambassador pod.
# We do this by checking that the pod's labels match the selector in the service.
for key, value in service_selector.items():
if len(selector) == 0:
return False

for key, value in selector.items():
pod_label_value = self.aconf.pod_labels.get(key)
if pod_label_value == value:
return True
if pod_label_value != value:
return False

return False
return True

def _process(self, obj: KubernetesObject) -> None:
# The annoying bit about K8s Service resources is that not only do we have to look
Expand All @@ -89,7 +97,7 @@ def _process(self, obj: KubernetesObject) -> None:
else:
self.discovered_services[obj.key] = obj

if self._is_ambassador_service(obj.labels, obj.spec.get('selector', {})):
if self._is_ambassador_service(obj):
self.logger.debug(f"Found Ambassador service: {obj.name}")
self.service_dep.ambassador_service = obj

Expand Down

0 comments on commit 1bbb412

Please sign in to comment.