kubernetes-sigs / cluster-api-provider-azure

Cluster API implementation for Microsoft Azure
https://capz.sigs.k8s.io/
Apache License 2.0
295 stars 424 forks source link

Make each of the flavors in available to deploy in Tilt #542

Closed devigned closed 4 years ago

devigned commented 4 years ago

/kind feature

Describe the solution you'd like [A clear and concise description of what you want to happen.]

When one runs make tilt-up, you are greeted with a Tilt console and browser window running a kind CAPZ management cluster, but there is no workload cluster running. It would be cool to be able to deploy any of the flavors under ./templates/cluster-template*.yaml as a local_resource via Tilt. It would give a nice visual experience for developers hacking on CAPZ.

As part of deploying one of the flavors, the tilt-settings file should probably offer overrides for environment variables which are included in the flavors, and provide sane default for the vars that are currently required (AZURE_LOCATION, CLUSTER_NAME, etc).

devigned commented 4 years ago

/help

devigned commented 4 years ago

/good-first-issue

muang0 commented 4 years ago

Hi all,

First time contributor here. I have tilt running in my development environment and am playing around with the Tiltfile.

For my clarification-- do we want this to be based on the tilt-settings.json itself? Or have the user run something like 'make tilt-up cluster-template-aks' to run default capz cluster + ./templates/cluster-template-aks.yaml worker cluster? (I am thinking the desired functionality is the former, but appreciate the direction)

Thanks for the help, James

muang0 commented 4 years ago

or do we want to use kustomize to deploy flavors from ./templates/flavors/*/ instead of ./templates/cluster-template*.yaml ?

alexeldeib commented 4 years ago

Based on the current workflow, I think the ask is to customize the Tiltfile + tilt-settings so when users execute make tilt-up, the end result is a workload cluster of their choice already being deployed (as selected by a value in tilt-settings, or the default). Sound like your first interpretation was correction.

e.g.:

{
  "template": "cluster-template-machinepool.yaml",
  "default_registry": "${REGISTRY}",
  "provider_repos": ["../cluster-api-provider-azure"],
  "enable_providers": ["azure", "docker", "kubeadm-bootstrap", "kubeadm-control-plane"],
  "kustomize_substitutions": {
      "AZURE_SUBSCRIPTION_ID_B64": "$(echo "${AZURE_SUBSCRIPTION_ID}" | tr -d '\n' | base64 | tr -d '\n')",
      "AZURE_TENANT_ID_B64": "$(echo "${AZURE_TENANT_ID}" | tr -d '\n' | base64 | tr -d '\n')",
      "AZURE_CLIENT_SECRET_B64": "$(echo "${AZURE_CLIENT_SECRET}" | tr -d '\n' | base64 | tr -d '\n')",
      "AZURE_CLIENT_ID_B64": "$(echo "${AZURE_CLIENT_ID}" | tr -d '\n' | base64 | tr -d '\n')"
  }
}
devigned commented 4 years ago

Originally, I had intended for each flavor to be a tilt local resource with a manual trigger. That way, each flavor would show up in the tilt UI. Thoughts?

alexeldeib commented 4 years ago

Does reloading the clusters make sense? We might need to do create-delete-create, or am I overthinking it?

alexeldeib commented 4 years ago

I should have caveated I hadn't discussed with you :) I was just noticing while flipping between the options it's a bit manual to test each one (or several at once especially).

Maybe we populate the variables programatically somehow to avoid collisions and stuff so we can template them all at once?

muang0 commented 4 years ago

qq- should I be referring to ./templates/flavors/*/ or ./templates/cluster-template*.yaml for sourcing the workloads?

devigned commented 4 years ago

I’m not sure the best flow... I’m totally open to ideas to help folks dev.

cluster-template*.yaml are the generated flavors, so using those would make sense. I could also see making a case for running a kustomize task on a flavor directoy.

devigned commented 4 years ago

@alexeldeib i really like your thoughts on allowing the different flavors to be deployed without collisions.

muang0 commented 4 years ago

Thoughts on a tilt-settings structure like below? Or am I over-complicating this you think?

Precedence could look something like:

  1. explicitly defined vars for each flavor i.e. worker-templates.flavors[0].AZURE_VNET_NAME
  2. vars defined at 'metadata' level-- spans workers i.e. metadata.AZURE_VNET_NAME
  3. programmatically defined default vars i.e. everything except azure tenant, client, subscription
{
    "kustomize_substitutions": {
        "AZURE_SUBSCRIPTION_ID_B64": "****",
        "AZURE_TENANT_ID_B64": "****",
        "AZURE_CLIENT_SECRET_B64": "****",
        "AZURE_CLIENT_ID_B64": "****"
    },
    "worker-templates": {
        "flavors": [{
            "base": {
                "CLUSTER_NAME": "test-cluster-name",
                "AZURE_LOCATION": "eastus",
                "AZURE_VNET_NAME": "test-vnet-name"
            },
            "system-assigned-identity": {
                "CLUSTER_NAME": "test-cluster-name-two",
                "AZURE_LOCATION": "westus",
                "AZURE_VNET_NAME": "test-vnet-name-two"
            }
        }],
        "metadata": {
            "AZURE_LOCATION": "eastus",
            "AZURE_VNET_NAME": "test-vnet-name",
            "AZURE_RESOURCE_GROUP": "test-resource-group-name",
            "AZURE_SUBSCRIPTION_ID": "****",
            "AZURE_TENANT_ID": "****",
            "AZURE_CLIENT_ID": "****",
            "AZURE_CLIENT_SECRET": "****",
            "CONTROL_PLANE_MACHINE_COUNT": "1",
            "KUBERNETES_VERSION": "v1.18.3",
            "AZURE_SSH_PUBLIC_KEY": "****",
            "AZURE_CONTROL_PLANE_MACHINE_TYPE": "Standard_D2s_v3",
            "WORKER_MACHINE_COUNT": "2",
            "AZURE_NODE_MACHINE_TYPE": "Standard_D2s_v3"
        }
    }
}

Also what about basing programmatically determined vars on the name of the flavor being deployed? for example a 'base' worker deployment would have CLUSTER_NAME along the lines of "worker-base" (unless defined explicitly in tilt-settings)? thinking about how to avoid conflicts in multi-worker deployments

devigned commented 4 years ago

@jroden, +1 to having programatic defaults. Great idea!

@vincepri, @ncdc, @detiber have you all thought about making flavors more accessible in tilt for CAPI? If so, do you have any opinion on a tilt-settings.json structure?

muang0 commented 4 years ago

Thanks! @devigned

A couple questions for anyone: should we have the tilt resource trigger_mode as auto or manual by default? Or should we have this setting also be part of a precedence pattern?

In the case where a user has tilt running and is modifying dependency files for a worker template-- Should we apply changes to the live worker cluster or tear down & rebuild? I am wondering about the case where user might try to change a spec that can't be modified

ncdc commented 4 years ago

FYI, we built our Tiltfile and tilt-settings.json before https://docs.tilt.dev/tiltfile_config.html existed. It might we worth exploring this newer functionality for something like a tilt up base or tilt up system-assigned-identity.

I've also thought about using this to specify which repos to deploy, e.g. tilt up core cabpk kcp capa or whatnot.

devigned commented 4 years ago

I would vote for flavors to be manual by default.

Perhaps, tear down would be handled manually by someone running tilt down ${flavor}?

muang0 commented 4 years ago

Ok, so I've made some progress and would appreciate some feedback :)

Functionality I have sofar:

Functionality I still need to figure out:

tilt screenshot: image

devigned commented 4 years ago

The UI flavors look awesome!

Heads up though, "base" is not a generated flavor, it is omitted and used for a base set of resources which other flavors can consume.

Can't wait to see the PR. TY!

muang0 commented 4 years ago

Would ./templates/cluster-template.yaml be a template we want to include as a flavor option?

devigned commented 4 years ago

Yes. That corresponds to the default flavor. See the flavor generation script.

I lean toward targeting ./template/cluster-template*.yaml files. The local resource should watch the flavor directory for changes, then run make generate-flavors to generate the ./template/cluster-template*.yaml with the changes from the flavors dir.

What do you think, @jroden?

muang0 commented 4 years ago

Thanks for the help! @devigned That sounds like a great workflow-- I appreciate the clarification (I'm new to the project so doing best to follow guidance while I get more familiar with use cases)

muang0 commented 4 years ago

@devigned I have the spin-up functionality configured and am working on a way for users to gracefully tear down worker-clusters.

I'm having trouble getting tilt down ${resource} to delete the worker clusters, and based on what I'm seeing in their documentation I'm not sure this fits our _local_resource/ tiltconfig use case. What about a make command to delete any running worker clusters, or maybe baking the delete process into make delete-cluster or make kind-reset ?

devigned commented 4 years ago

Don’t worry about tear down for now. We can always build upon the experience in subsequent PRs.

muang0 commented 4 years ago

sounds good! I will review the code & PR process and move forward to that stage!

vincepri commented 4 years ago

As long as you know the cluster name, you can always do kubectl delete cluster <name>.

CecileRobertMichon commented 4 years ago

/assign @jroden /remove-help