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

Support deploying multiple ingresses #98

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
91 changes: 69 additions & 22 deletions fiaas_deploy_daemon/deployer/kubernetes/ingress.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,14 @@
from itertools import chain

from k8s.client import NotFound
from k8s.base import Equality, Inequality, Exists
from k8s.models.common import ObjectMeta
from k8s.models.ingress import Ingress, IngressSpec, IngressRule, HTTPIngressRuleValue, HTTPIngressPath, IngressBackend, \
IngressTLS

from fiaas_deploy_daemon.retry import retry_on_upsert_conflict
from fiaas_deploy_daemon.tools import merge_dicts
from collections import namedtuple

LOG = logging.getLogger(__name__)

Expand All @@ -43,48 +45,89 @@ def deploy(self, app_spec, labels):
if self._should_have_ingress(app_spec):
self._create(app_spec, labels)
else:
self.delete(app_spec)
self._delete_unused(app_spec, labels)

def delete(self, app_spec):
LOG.info("Deleting ingress for %s", app_spec.name)
LOG.info("Deleting ingresses for %s", app_spec.name)
try:
Ingress.delete(app_spec.name, app_spec.namespace)
Ingress.delete_list(namespace=app_spec.namespace, labels={"app": Equality(app_spec.name), "fiaas/deployment_id": Exists()})
except NotFound:
pass

@retry_on_upsert_conflict
def _create(self, app_spec, labels):
LOG.info("Creating/updating ingress for %s", app_spec.name)
annotations = {
u"fiaas/expose": u"true" if _has_explicitly_set_host(app_spec) else u"false"
LOG.info("Creating/updating ingresses for %s", app_spec.name)
custom_labels = merge_dicts(app_spec.labels.ingress, labels)

# Group app_spec.ingresses to separate those with annotations
AnnotatedIngress = namedtuple("AnnotatedIngress", ["name", "ingress_items", "annotations"])
Copy link
Member

Choose a reason for hiding this comment

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

Maybe extracting the grouping logic to a separate function would make this section read a little better?

unannotated_ingress = AnnotatedIngress(name=app_spec.name, ingress_items=[], annotations={})
ingresses_by_annotations = [unannotated_ingress]
Copy link
Member

Choose a reason for hiding this comment

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

nit :The naming here confused me a bit; maybe something like all_ingresses or just ingresses would read better?

for ingress_item in app_spec.ingresses:
LOG.info(ingress_item)
Copy link
Contributor

Choose a reason for hiding this comment

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

It was your intention to only log the item here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, no. Left over from debugging: removed in 98bd437

if ingress_item.annotations:
next_name = "{}-{}".format(app_spec.name, len(ingresses_by_annotations))
annotated_ingresses = AnnotatedIngress(name=next_name, ingress_items=[ingress_item], annotations=ingress_item.annotations)
ingresses_by_annotations.append(annotated_ingresses)
else:
unannotated_ingress.ingress_items.append(ingress_item)

LOG.info("Will create %s ingresses", len(ingresses_by_annotations))
for annotated_ingress in ingresses_by_annotations:
if len(annotated_ingress.ingress_items) == 0:
LOG.info("No items, skipping: %s", annotated_ingress)
continue

self._create_ingress(app_spec, annotated_ingress, custom_labels)

self._delete_unused(app_spec, custom_labels)

@retry_on_upsert_conflict
def _create_ingress(self, app_spec, annotated_ingress, labels):
default_annotations = {
u"fiaas/expose": u"true" if _has_explicitly_set_host(annotated_ingress.ingress_items) else u"false"
}
annotations = merge_dicts(annotated_ingress.annotations, app_spec.annotations.ingress, default_annotations)

custom_labels = merge_dicts(app_spec.labels.ingress, labels)
custom_annotations = merge_dicts(app_spec.annotations.ingress, annotations)
metadata = ObjectMeta(name=app_spec.name, namespace=app_spec.namespace, labels=custom_labels,
annotations=custom_annotations)
metadata = ObjectMeta(name=annotated_ingress.name, namespace=app_spec.namespace, labels=labels,
annotations=annotations)

per_host_ingress_rules = [
IngressRule(host=self._apply_host_rewrite_rules(ingress_item.host),
http=self._make_http_ingress_rule_value(app_spec, ingress_item.pathmappings))
for ingress_item in app_spec.ingresses
for ingress_item in annotated_ingress.ingress_items
if ingress_item.host is not None
]
default_host_ingress_rules = self._create_default_host_ingress_rules(app_spec)
if annotated_ingress.annotations:
use_suffixes = False
host_ingress_rules = per_host_ingress_rules
else:
use_suffixes = True
host_ingress_rules = per_host_ingress_rules + self._create_default_host_ingress_rules(app_spec)

ingress_spec = IngressSpec(rules=per_host_ingress_rules + default_host_ingress_rules)
ingress_spec = IngressSpec(rules=host_ingress_rules)

ingress = Ingress.get_or_create(metadata=metadata, spec=ingress_spec)
self._ingress_tls.apply(ingress, app_spec, self._get_hosts(app_spec))

hosts_for_tls = [rule.host for rule in host_ingress_rules]
self._ingress_tls.apply(ingress, app_spec, hosts_for_tls, use_suffixes=use_suffixes)
self._owner_references.apply(ingress, app_spec)
ingress.save()

def _delete_unused(self, app_spec, labels):
filter_labels = [
("app", Equality(labels["app"])),
("fiaas/deployment_id", Exists()),
("fiaas/deployment_id", Inequality(labels["fiaas/deployment_id"]))
]
Ingress.delete_list(namespace=app_spec.namespace, labels=filter_labels)

def _generate_default_hosts(self, name):
for suffix in self._ingress_suffixes:
yield u"{}.{}".format(name, suffix)

def _create_default_host_ingress_rules(self, app_spec):
all_pathmappings = chain.from_iterable(ingress_item.pathmappings for ingress_item in app_spec.ingresses)
all_pathmappings = chain.from_iterable(ingress_item.pathmappings
for ingress_item in app_spec.ingresses if not ingress_item.annotations)
http_ingress_rule_value = self._make_http_ingress_rule_value(app_spec, all_pathmappings)
return [IngressRule(host=host, http=http_ingress_rule_value)
for host in self._generate_default_hosts(app_spec.name)]
Expand All @@ -99,7 +142,7 @@ def _should_have_ingress(self, app_spec):
return self._can_generate_host(app_spec) and _has_ingress(app_spec) and _has_http_port(app_spec)

def _can_generate_host(self, app_spec):
return len(self._ingress_suffixes) > 0 or _has_explicitly_set_host(app_spec)
return len(self._ingress_suffixes) > 0 or _has_explicitly_set_host(app_spec.ingresses)

@staticmethod
def _make_http_ingress_rule_value(app_spec, pathmappings):
Expand All @@ -115,8 +158,8 @@ def _get_hosts(self, app_spec):
for ingress_item in app_spec.ingresses if ingress_item.host is not None]


def _has_explicitly_set_host(app_spec):
return any(ingress.host is not None for ingress in app_spec.ingresses)
def _has_explicitly_set_host(ingress_items):
return any(ingress_item.host is not None for ingress_item in ingress_items)


def _has_http_port(app_spec):
Expand All @@ -142,7 +185,7 @@ def __init__(self, config):
self._shortest_suffix = sorted(config.ingress_suffixes, key=len)[0] if config.ingress_suffixes else None
self.enable_deprecated_tls_entry_per_host = config.enable_deprecated_tls_entry_per_host

def apply(self, ingress, app_spec, hosts):
def apply(self, ingress, app_spec, hosts, use_suffixes=True):
if self._should_have_ingress_tls(app_spec):
tls_annotations = {}
if self._cert_issuer or app_spec.ingress_tls.certificate_issuer:
Expand All @@ -162,8 +205,12 @@ def apply(self, ingress, app_spec, hosts):
else:
ingress.spec.tls = []

collapsed = self._collapse_hosts(app_spec, hosts)
ingress.spec.tls.append(IngressTLS(hosts=collapsed, secretName="{}-ingress-tls".format(app_spec.name)))
if use_suffixes:
# adding app-name to suffixes could result in a host too long to be the common-name of a cert, and
# as the user doesn't control it we should generate a host we know will fit
hosts = self._collapse_hosts(app_spec, hosts)

ingress.spec.tls.append(IngressTLS(hosts=hosts, secretName="{}-ingress-tls".format(ingress.metadata.name)))

def _collapse_hosts(self, app_spec, hosts):
"""The first hostname in the list will be used as Common Name in the certificate"""
Expand Down
1 change: 1 addition & 0 deletions fiaas_deploy_daemon/specs/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ def version(self):
IngressItemSpec = namedtuple("IngressItemSpec", [
"host",
"pathmappings",
"annotations"
])

IngressPathMappingSpec = namedtuple("IngressPathMappingSpec", [
Expand Down
1 change: 1 addition & 0 deletions fiaas_deploy_daemon/specs/v3/defaults.yml
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ ingress: # Generate ingress rules for access from outside cluster. To disable, s
paths: # List of paths exposed to which application port
- path: / # Path the application answers on
port: http # Name of the port path is served on
annotations: {}
healthchecks: # Healthchecks defined for your application. If omitted and a single port is defined, liveness will default to http or tcp depending on port type, and readiness will be a copy of liveness. If no ports or multiple ports are defined, healthchecks are not provided and should be defined explicitly
liveness:
# Valid configuration requires exactly one of execute|http|tcp
Expand Down
6 changes: 3 additions & 3 deletions fiaas_deploy_daemon/specs/v3/factory.py
Original file line number Diff line number Diff line change
Expand Up @@ -207,15 +207,15 @@ def resolve_port_number(port):
else:
raise InvalidConfiguration("{} is not a valid port name or port number".format(port))

def ingress_item(host, paths):
def ingress_item(host, paths, annotations):
ingress_path_mapping_specs = [
IngressPathMappingSpec(path=pathmapping["path"], port=resolve_port_number(pathmapping["port"]))
for pathmapping in paths
]
return IngressItemSpec(host=host, pathmappings=ingress_path_mapping_specs)
return IngressItemSpec(host=host, pathmappings=ingress_path_mapping_specs, annotations=annotations)

if len(http_ports.items()) > 0:
return [ingress_item(host_path_mapping["host"], host_path_mapping["paths"])
return [ingress_item(host_path_mapping["host"], host_path_mapping["paths"], host_path_mapping["annotations"])
for host_path_mapping in ingress_lookup]
else:
return []
Expand Down
2 changes: 1 addition & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ def read(filename):
"pinject == 0.14.1",
"six == 1.12.0",
"dnspython == 1.16.0",
"k8s == 0.14.0",
"k8s == 0.15.0",
"monotonic == 1.5",
"appdirs == 1.4.3",
"requests-toolbelt == 0.9.1",
Expand Down
4 changes: 2 additions & 2 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,11 @@ def prometheus_registry():


@pytest.helpers.register
def assert_any_call(mockk, first, *args):
def assert_any_call(mockk, first, *args, **kwargs):
__tracebackhide__ = True

def _assertion():
mockk.assert_any_call(first, *args)
mockk.assert_any_call(first, *args, **kwargs)

_add_useful_error_message(_assertion, mockk, first, args)

Expand Down
2 changes: 1 addition & 1 deletion tests/fiaas_deploy_daemon/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ def app_spec():
deployment_id="test_app_deployment_id",
labels=LabelAndAnnotationSpec({}, {}, {}, {}, {}, {}),
annotations=LabelAndAnnotationSpec({}, {}, {}, {}, {}, {}),
ingresses=[IngressItemSpec(host=None, pathmappings=[IngressPathMappingSpec(path="/", port=80)])],
ingresses=[IngressItemSpec(host=None, pathmappings=[IngressPathMappingSpec(path="/", port=80)], annotations={})],
strongbox=StrongboxSpec(enabled=False, iam_role=None, aws_region="eu-west-1", groups=None),
singleton=False,
ingress_tls=IngressTlsSpec(enabled=False, certificate_issuer=None),
Expand Down
Loading