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 for pinning the IP address of the load balancer via terraform overrides #1235

Merged
merged 3 commits into from
Apr 11, 2022
Merged
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
30 changes: 30 additions & 0 deletions docs/source/installation/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -690,6 +690,36 @@ jupyterhub:
users: true
```

## Terraform Overrides

The QHub configuration file provides a huge number of configuration options for customizing your
QHub Infrastructure, while these options are sufficient for an average user, but aren't
exhaustive by any means. There are still a plenty of things you might want to achieve which
cannot be configured directly by the above mentioned options, hence we've introduced a
new option called terraform overrides (`terraform_overrides`), which lets you override
the values of terraform variables in specific modules/resource. This is a relatively
advance feature and must be used with utmost care and you should really know, what
you're doing.

Here we describe the overrides supported via QHub config file:

### Ingress

You can configure the IP of the load balancer and add annotations for the same via `ingress`'s
terraform overrides, one such example for GCP is:


```yaml
ingress:
terraform_overrides:
load_balancer_annotations:
"networking.gke.io/load-balancer-type": "Internal"
"networking.gke.io/internal-load-balancer-subnet": "pre-existing-subnet"
load_balancer_ip: "1.2.3.4"
```

This is quite useful for pinning the IP Address of the load balancer.

# Full configuration example

```yaml
Expand Down
10 changes: 10 additions & 0 deletions qhub/schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -382,6 +382,15 @@ class QHubExtension(Base):
envs: typing.Optional[typing.List[QHubExtensionEnv]]


class IngressTerraformOverrides(Base):
load_balancer_annotations: typing.Optional[typing.Dict]
load_balancer_ip: str


class Ingress(Base):
terraform_overrides: IngressTerraformOverrides
Copy link
Member

Choose a reason for hiding this comment

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

I'd prefer if this is terraform_overrides: Any and we defer to terraform for the checks.

Copy link
Member

Choose a reason for hiding this comment

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

So no need for the IngressTerraformOverrides

Copy link
Member Author

Choose a reason for hiding this comment

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

The problem with this is, "Any" won't reach terraform, as we parse these in the input_vars.py, so anything other than this won't be passed, unless we pass them explicitly.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, I see your comment below now.



# ======== External Container Registry ========

# This allows the user to set a private AWS ECR as a replacement for
Expand Down Expand Up @@ -455,6 +464,7 @@ class Main(Base):
prevent_deploy: bool = (
False # Optional, but will be given default value if not present
)
ingress: typing.Optional[Ingress]

# If the qhub_version in the schema is old
# we must tell the user to first run qhub upgrade
Expand Down
7 changes: 7 additions & 0 deletions qhub/stages/input_vars.py
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,9 @@ def _calculate_note_groups(config):


def stage_04_kubernetes_ingress(stage_outputs, config):
ingress_terraform_overrides = config.get("ingress", {}).get(
"terraform_overrides", {}
)
return {
"name": config["project_name"],
"environment": config["namespace"],
Expand All @@ -152,6 +155,10 @@ def stage_04_kubernetes_ingress(stage_outputs, config):
"certificate-secret-name": config["certificate"]["secret_name"]
if config["certificate"]["type"] == "existing"
else None,
"load-balancer-annotations": ingress_terraform_overrides.get(
Copy link
Member

Choose a reason for hiding this comment

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

Using what I showed above we just supply terraform_overrides e.g. **config.get("ingress", {}).get("terraform_overrides": {}). Terraform variables will do all the checking for us.

Copy link
Member Author

Choose a reason for hiding this comment

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

The problem is, we need load-balancer-ip and load-balancer-annotations separately, to be passed to different variables, so that they can be used at their designated places:

In qhub/template/stages/04-kubernetes-ingress/modules/kubernetes/ingress/main.tf

spec:

  spec {
    selector = merge({
      "app.kubernetes.io/component" = "traefik-ingress"
    }, var.load-balancer-annotations)

and

    type = "LoadBalancer"
    load_balancer_ip = var.load-balancer-ip

Copy link
Member

Choose a reason for hiding this comment

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

I think this should work. Suppose

ingress:
  terraform_overrides:
      load-balancer-annotations:
           k1: v1
           k2: v2
      load-balancer-ip: 1.2.3.4

Then in input-vars.py we supply:

    return {
        "name": config["project_name"],
        "environment": config["namespace"],
        "node_groups": _calculate_note_groups(config),
        "enable-certificates": (config["certificate"]["type"] == "lets-encrypt"),
        "acme-email": config["certificate"].get("acme_email"),
        "acme-server": config["certificate"].get("acme_server"),
        "certificate-secret-name": config["certificate"]["secret_name"]
        if config["certificate"]["type"] == "existing"
        else None,
        **config.get("ingress", {}).get("terraform_overrides": {})
    }

This way terraform_overrides key can override any setting. Does this make sense?

Copy link
Member Author

Choose a reason for hiding this comment

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

This way terraform_overrides key can override any setting.

Yes, any variable present in the terraform. This makes more sense. I'll add this.

"load_balancer_annotations", {}
),
"load-balancer-ip": ingress_terraform_overrides.get("load-balancer-ip"),
}


Expand Down
10 changes: 6 additions & 4 deletions qhub/template/stages/04-kubernetes-ingress/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,10 @@ module "kubernetes-ingress" {

node-group = var.node_groups.general

enable-certificates = var.enable-certificates
acme-email = var.acme-email
acme-server = var.acme-server
certificate-secret-name = var.certificate-secret-name
enable-certificates = var.enable-certificates
acme-email = var.acme-email
acme-server = var.acme-server
certificate-secret-name = var.certificate-secret-name
load-balancer-annotations = var.load-balancer-annotations
load-balancer-ip = var.load-balancer-ip
}
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,9 @@ resource "kubernetes_service" "main" {
}

spec {
selector = {
selector = merge({
"app.kubernetes.io/component" = "traefik-ingress"
}
}, var.load-balancer-annotations)

port {
name = "http"
Expand Down Expand Up @@ -111,6 +111,7 @@ resource "kubernetes_service" "main" {
}

type = "LoadBalancer"
load_balancer_ip = var.load-balancer-ip
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,3 +58,18 @@ variable "certificate-secret-name" {
type = string
default = null
}

variable "load-balancer-ip" {
description = "IP Address of the load balancer"
type = string
default = null
}

variable "load-balancer-annotations" {
description = "Annotations for the load balancer"
type = map(object({
key = string
value = string
}))
default = null
}
17 changes: 17 additions & 0 deletions qhub/template/stages/04-kubernetes-ingress/variables.tf
Original file line number Diff line number Diff line change
Expand Up @@ -38,3 +38,20 @@ variable "certificate-secret-name" {
description = "Kubernetes secret used for certificate"
default = ""
}


variable "load-balancer-ip" {
description = "IP Address of the load balancer"
type = string
default = null
}


variable "load-balancer-annotations" {
description = "Annotations for the load balancer"
type = map(object({
key = string
value = string
}))
default = null
}