Introduce improvements to VIP mechanism

README.md

@@ -25,15 +25,15 @@ ubuntu@infra{1,2,3}:~$ newgrp snap_daemon
 
 ### Bootstrap the first node
 
-Note: You will be asked for a `virtual_ip` during installation of the HAProxy charm, `accept-defaults` does not modify this behaviour.
-Pass an empty value to disable it, or any valid IP to enable; the Keepalived charm will be installed to enable connecting to HA MAAS using the VIP.
-
 ```bash
 ubuntu@infra1:~$ maas-anvil cluster bootstrap \
     --role database --role region --role agent --role haproxy \
     --accept-defaults
 ```
 
+Note: You will be asked for a `virtual_ip` during installation of the HAProxy charm, if `accept-defaults` is omitted.
+Pass an empty value to disable it, or any valid IP to enable; the Keepalived charm will be installed to enable connecting to HA MAAS using the VIP.
+
 ### Add new nodes to the MAAS cluster
 
 ```bash

anvil-python/anvil/commands/haproxy.py

@@ -13,33 +13,25 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+import ipaddress
 import logging
 from typing import Any, List
 
 from rich.console import Console
 from sunbeam.clusterd.client import Client
 from sunbeam.commands.terraform import TerraformInitStep
+from sunbeam.jobs import questions
+from sunbeam.jobs.common import BaseStep
 from sunbeam.jobs.juju import JujuHelper
-from sunbeam.jobs.manifest import BaseStep
-from sunbeam.jobs.questions import (
-    PromptQuestion,
-    QuestionBank,
-    load_answers,
-    write_answers,
-)
 from sunbeam.jobs.steps import (
     AddMachineUnitsStep,
     DeployMachineApplicationStep,
     RemoveMachineUnitStep,
 )
 
-from anvil.jobs.common import (
-    validate_ip_address,
-)
 from anvil.jobs.manifest import Manifest
 
 LOG = logging.getLogger(__name__)
-from anvil.provider.local.deployment import LocalDeployment
 
 APPLICATION = "haproxy"
 CONFIG_KEY = "TerraformVarsHaproxyPlan"
@@ -50,24 +42,41 @@
     1200  # 15 minutes, adding / removing units can take a long time
 )
 
-CONF_VAR = dict[str, dict[str, Any]]
+
+def keepalived_questions() -> dict[str, questions.PromptQuestion]:
+    return {
+        "virtual_ip": questions.PromptQuestion(
+            "Virtual IP to use for the Cluster in HA",
+            default_value="",
+            validation_function=validate_virtual_ip,
+        ),
+    }
 
 
-def keepalived_questions() -> dict[str, Any]:
-    return {"virtual_ip": "", "vip_hostname": "", "port": 443}
+def validate_virtual_ip(value: str) -> str:
+    """We allow passing an empty IP for virtual_ip"""
+    if value == "":
+        return ""
+    try:
+        return ipaddress.ip_address(value).exploded
+    except ValueError as e:
+        raise ValueError(f"{value} is not a valid IP address: {e}")
 
 
 class DeployHAProxyApplicationStep(DeployMachineApplicationStep):
     """Deploy HAProxy application using Terraform"""
 
+    _KEEPALIVED_CONFIG = KEEPALIVED_CONFIG_KEY
+
     def __init__(
         self,
         client: Client,
         manifest: Manifest,
         jhelper: JujuHelper,
         model: str,
-        refresh: bool = False,
+        deployment_preseed: dict[Any, Any] | None = None,
         accept_defaults: bool = False,
+        refresh: bool = False,
     ):
         super().__init__(
             client,
@@ -81,7 +90,7 @@ def __init__(
             "Deploying HAProxy",
             refresh,
         )
-        self.variables: CONF_VAR = {}
+        self.preseed = deployment_preseed or {}
         self.accept_defaults = accept_defaults
 
     def get_application_timeout(self) -> int:
@@ -91,46 +100,35 @@ def has_prompts(self) -> bool:
         return True
 
     def prompt(self, console: Console | None = None) -> None:
-        self.variables = load_answers(self.client, KEEPALIVED_CONFIG_KEY)
-        self.variables.setdefault("keepalived_config", {})
-        self.variables["keepalived_config"].setdefault("virtual_ip", "")
-        self.variables["keepalived_config"].setdefault("vip_hostname", "")
-        self.variables["keepalived_config"].setdefault("port", 443)
-
-        keepalive_bank = QuestionBank(
-            questions={
-                "virtual_ip": PromptQuestion(
-                    "Virtual IP to use for the Cluster in HA",
-                    default_value=keepalived_questions().get("virtual_ip"),
-                    validation_function=validate_ip_address,
-                ),
-                "vip_hostname": PromptQuestion(
-                    "Virtual IP hostname for the Cluster in HA",
-                    default_value=keepalived_questions().get("vip_hostname"),
-                ),
-                "port": PromptQuestion(
-                    "Virtual IP port to use for the Cluster in HA",
-                    default_value=keepalived_questions().get("port"),
-                ),
-            },
+        variables = questions.load_answers(
+            self.client, self._KEEPALIVED_CONFIG
+        )
+        variables.setdefault("virtual_ip", "")
+
+        # Set defaults
+        self.preseed.setdefault("virtual_ip", "")
+
+        keepalived_config_bank = questions.QuestionBank(
+            questions=keepalived_questions(),
             console=console,
-            previous_answers=self.variables,
+            preseed=self.preseed,
+            previous_answers=variables,
             accept_defaults=self.accept_defaults,
         )
 
-        self.variables["keepalived_config"]["virtual_ip"] = (
-            keepalive_bank.virtual_ip.ask()
-        )
-        self.variables["keepalived_config"]["vip_hostname"] = (
-            keepalive_bank.vip_hostname.ask()
-        )
-        self.variables["keepalived_config"]["port"] = keepalive_bank.port.ask()
+        variables["virtual_ip"] = keepalived_config_bank.virtual_ip.ask()
 
-        LOG.debug(self.variables)
-        write_answers(self.client, CONFIG_KEY, self.variables)
+        LOG.debug(variables)
+        questions.write_answers(
+            self.client, self._KEEPALIVED_CONFIG, variables
+        )
 
     def extra_tfvars(self) -> dict[str, Any]:
-        return self.variables
+        variables: dict[str, Any] = questions.load_answers(
+            self.client, self._KEEPALIVED_CONFIG
+        )
+        variables["haproxy_port"] = 80
+        return variables
 
 
 class AddHAProxyUnitsStep(AddMachineUnitsStep):
@@ -183,20 +181,20 @@ def haproxy_install_steps(
     client: Client,
     manifest: Manifest,
     jhelper: JujuHelper,
-    deployment: LocalDeployment,
+    model: str,
     fqdn: str,
-    accept_defaults: bool = False,
+    accept_defaults: bool,
+    preseed: dict[Any, Any],
 ) -> List[BaseStep]:
     return [
         TerraformInitStep(manifest.get_tfhelper("haproxy-plan")),
         DeployHAProxyApplicationStep(
             client,
             manifest,
             jhelper,
-            deployment.infrastructure_model,
+            model,
             accept_defaults=accept_defaults,
+            deployment_preseed=preseed,
         ),
-        AddHAProxyUnitsStep(
-            client, fqdn, jhelper, deployment.infrastructure_model
-        ),
+        AddHAProxyUnitsStep(client, fqdn, jhelper, model),
     ]

anvil-python/anvil/jobs/common.py

@@ -14,8 +14,7 @@
 # limitations under the License.
 
 import enum
-import ipaddress
-from typing import Any, Optional
+from typing import Any
 
 import click
 
@@ -79,13 +78,3 @@ def validate_roles(
         return [Role[role.upper()] for role in value]
     except KeyError as e:
         raise click.BadParameter(str(e))
-
-
-def validate_ip_address(value: str) -> str:
-    """We allow passing an empty IP for Virtual_ip"""
-    if value == "":
-        return ""
-    try:
-        return ipaddress.ip_address(value).exploded
-    except ValueError as e:
-        raise click.BadParameter(f"{value} is not a valid IP address: {e}")

anvil-python/anvil/provider/local/commands.py

@@ -272,12 +272,12 @@ def bootstrap(
                 client,
                 manifest_obj,
                 jhelper,
-                deployment,
+                deployment.infrastructure_model,
                 fqdn,
                 accept_defaults,
+                preseed,
             )
         )
-
     if is_region_node:
         plan4.extend(
             maas_region_install_steps(
@@ -354,6 +354,7 @@ def _print_output(token: str) -> None:
 
 
 @click.command()
+@click.option("-a", "--accept-defaults", help="Accept all defaults.", is_flag=True)
 @click.option("--token", type=str, help="Join token")
 @click.option(
     "--role",
@@ -371,6 +372,7 @@ def join(
     ctx: click.Context,
     token: str,
     roles: List[Role],
+    accept_defaults: bool = False,
 ) -> None:
     """Join node to the cluster.
 
@@ -419,6 +421,7 @@ def join(
     manifest_obj = Manifest.load_latest_from_clusterdb(
         deployment, include_defaults=True
     )
+    preseed = manifest_obj.deployment_config
 
     machine_id = -1
     machine_id_result = get_step_message(plan1_results, AddJujuMachineStep)
@@ -437,7 +440,13 @@ def join(
     if is_haproxy_node:
         plan2.extend(
             haproxy_install_steps(
-                client, manifest_obj, jhelper, deployment, name
+                client,
+                manifest_obj,
+                jhelper,
+                deployment.infrastructure_model,
+                name,
+                accept_defaults,
+                preseed,
             )
         )
     if is_region_node:

anvil-python/anvil/provider/local/deployment.py

@@ -21,15 +21,13 @@
     ClusterServiceUnavailableException,
 )
 from sunbeam.commands.juju import BOOTSTRAP_CONFIG_KEY, bootstrap_questions
-from sunbeam.jobs.questions import (
-    QuestionBank,
-    load_answers,
-    show_questions,
-)
+from sunbeam.jobs.questions import QuestionBank, load_answers, show_questions
 from sunbeam.provider.local.deployment import (
     LocalDeployment as SunbeamLocalDeployment,
 )
 
+from anvil.commands.haproxy import KEEPALIVED_CONFIG_KEY, keepalived_questions
+
 LOG = logging.getLogger(__name__)
 LOCAL_TYPE = "local"
 
@@ -55,5 +53,19 @@ def generate_preseed(self, console: Console) -> str:
             show_questions(bootstrap_bank, section="bootstrap")
         )
 
+        # HAProxy questions
+        try:
+            variables = load_answers(client, KEEPALIVED_CONFIG_KEY)
+        except ClusterServiceUnavailableException:
+            variables = {}
+        keepalived_config_bank = QuestionBank(
+            questions=keepalived_questions(),
+            console=console,
+            previous_answers=variables,
+        )
+        preseed_content.extend(
+            show_questions(keepalived_config_bank, section="haproxy")
+        )
+
         preseed_content_final = "\n".join(preseed_content)
         return preseed_content_final

cloud/etc/deploy-haproxy/main.tf

@@ -30,6 +30,10 @@ data "juju_model" "machine_model" {
   name = var.machine_model
 }
 
+locals {
+  virtual_ip = var.virtual_ip != "" ? { virtual_ip = var.virtual_ip } : {}
+}
+
 resource "juju_application" "haproxy" {
   name  = "haproxy"
   model = data.juju_model.machine_model.name
@@ -58,12 +62,9 @@ resource "juju_application" "keepalived" {
   }
 
   config = merge(
+    { port = var.haproxy_port },
+    local.virtual_ip,
     var.charm_keepalived_config,
-    {
-      "virtual_ip": var.virtual_ip,
-      "vip_hostname": var.vip_hostname,
-      "port": var.port,
-    }
   )
 }
 

cloud/etc/deploy-haproxy/variables.tf

@@ -55,25 +55,19 @@ variable "charm_keepalived_revision" {
 }
 
 variable "virtual_ip" {
-  description = "Virtual IP Address for keepalived"
+  description = "Virtual IP Address for Keepalived"
   type        = string
   default     = ""
 }
 
-variable "vip_hostname" {
-  description = "Virtual IP hostname for keepalived"
+variable "haproxy_port" {
+  description = "The port that HAProxy listens on"
   type        = string
-  default     = ""
-}
-
-variable "port" {
-  description = "Virtual IP port for keepalived clients"
-  type        = string
-  default     = ""
+  default     = 80
 }
 
 variable "charm_keepalived_config" {
   description = "Operator config for Keepalived deployment"
   type        = map(string)
   default     = {}
-}
+}