[mickmis]

See #1761 (comment)
This should fix #1761

• update tanka, helm, terraform deployments to determistically determine the minute of the eviction schedule based on the locality, in order to (unless set explicitly) randomly offset the time at which the instances will run the eviction
• update cleanup doc:
  • reorganize page by clearly separating general cleanup information from the tool usage
  • clarify eviction should be done by all pool members
  • slightly unrelated fix: add limit arguments

I am particularly interested in your review for this PR @BenjaminPelletier

Testing

Tanka

Uses md5, so different value than Helm and Terraform expected.
It does so because sha256 is not available in all versions of libsonnet.

tk show --dangerous-allow-redirect deploy/services/tanka/examples/minikube -t 'CronJob/.*'
...
schedule: 6 * * * *
...
schedule: 6 2 * * *

Helm

Uses sha256, because md5 is not supported.

mkdir -p /tmp/helm-test
cp -r deploy/services/helm-charts/dss/* /tmp/helm-test/
rm -f /tmp/helm-test/grafana_dashboards
sed -i '/^dependencies:/,$d' /tmp/helm-test/Chart.yaml
helm template test-release /tmp/helm-test \
      -s templates/dss-evict.yaml \
      --set dss.image=interuss/dss:v0.23.0 \
      --set dss.conf.hostname=dss.example.com \
      --set dss.conf.locality=minikube \
      --set global.cloudProvider=minikube \
      --set cockroachdb.conf.cluster-name=test \
      --set loadBalancers.dssGateway.ip=1.2.3.4 \
      --set "loadBalancers.cockroachdbNodes[0].ip=1.2.3.4"
# Source: dss/templates/dss-evict.yaml
apiVersion: batch/v1
kind: CronJob
metadata:
  labels:
    name: dss-scd-evict
  name: dss-scd-evict
  namespace: default
spec:
  schedule: "13 2 * * *"
...
# Source: dss/templates/dss-evict.yaml
apiVersion: batch/v1
kind: CronJob
metadata:
  labels:
    name: dss-rid-evict
  name: dss-rid-evict
  namespace: default
spec:
  schedule: "13 * * * *"
...

Terraform

cp -r deploy/infrastructure/dependencies/terraform-commons-dss /tmp/tf-test

# Convert local_file resources into outputs so `terraform apply` prints them directly without needing provider init:
python3 -c '
    import re
    with open("/tmp/tf-test/helm.tf") as f: s = f.read()
    s = s.replace("resource \"local_file\" \"helm_chart_values\" {\n  filename = \"${local.
  workspace_location}/helm_values.yml\"\n  content =", "output \"helm_chart_values\" {\n  value =")
    with open("/tmp/tf-test/helm.tf", "w") as f: f.write(s)

    with open("/tmp/tf-test/tanka.tf") as f: s = f.read()
    s = re.sub(r"resource \"local_file\" \"tanka_config_main\" \{\s*content =", "output \"tanka_config_main\" {\n
  value =", s)
    s = re.sub(r"\n\s*filename = \"\$\{local\.tanka_workspace_location\}/main\.jsonnet\"\n\}", "\n}", s)
    s = re.sub(r"resource \"local_file\" \"tanka_config_spec\" \{.*", "", s, flags=re.DOTALL)
    with open("/tmp/tf-test/tanka.tf", "w") as f: f.write(s)
 '
rm -f /tmp/tf-test/scripts.tf /tmp/tf-test/output.tf

cat << 'EOF' > /tmp/tf-test/test.tfvars
    app_hostname                    = "dss.example.com"
    db_hostname_suffix              = "db.example.com"
    cluster_name                    = "test-cluster"
    crdb_cluster_name               = "test-crdb"
    crdb_image_tag                  = "v24.1.3"
    node_count                      = 3
    should_init                     = true
    image                           = "interuss/dss:v0.23.0"
    authorization                   = { public_key_pem_path = "/test-certs/auth2.pem" }
    locality                        = "minikube"
    kubernetes_cloud_provider_name  = "google"
    kubernetes_get_credentials_cmd  = "echo ok"
    kubernetes_context_name         = "test-ctx"
    kubernetes_api_endpoint         = "https://127.0.0.1/"
    crdb_internal_nodes             = [{ dns = "0.db.example.com", ip = "1.2.3.4" }]
    yugabyte_internal_masters_nodes = []
    yugabyte_internal_tservers_nodes = []
    ip_gateway                      = "1.2.3.5"
    ip_prometheus                   = "1.2.3.6"
    kubernetes_storage_class        = "standard"
EOF

terraform -chdir=/tmp/tf-test apply -auto-approve -var-file=/tmp/tf-test/test.tfvars

...
  evict+: {
    scd+: {
      enable_cron: false,
      schedule: "13 2 * * *",
      ttl: "2688h",
      timeout: "",
      limit: 0,
      operational_intents: true,
      subscriptions: true,
    },
    rid+: {
      enable_cron: true,
      schedule: "13 * * * *",
      ttl: "30m",
      timeout: "",
      limit: 0,
      ISAs: true,
      subscriptions: true,
    },
...

[BenjaminPelletier]

Is this a supported method of deployment configuration? This might be a broader question of how a user makes modifications to an existing deployment (for instance, changing the RID evict schedule, but more generally any of the tfvars); I don't see this user journey ("I want to alter a characteristic of my already-deployed DSS instance") addressed by our current operations documentation.

[mickmis]

That's a good point. I propose to leave that as is in this PR and open an issue tracking the more general documentation question, with a link to this documentation page to not forget updating it once it will be addressed.

[BenjaminPelletier]

Perhaps not necessary in this PR, but it seems like we should fix this warning. It seems unreasonable to recommend a backup of state data (which is what the DSS contains), and it seems unnecessary to review and validate the list of entities identified as expired (is there any substantive risk that non-expired entities would be identified? would we really review and validate the list of entities if we were running this command?)