Support templating Openstack clusters as App CRs #635

ljakimczuk · 2022-01-17T13:53:43Z

Description

The kubectl gs template cluster --provider openstack is now able to output clusters as App CRs. It does that by running two additional commands underneath, one for templating the cluste-openstack app and another for templating the default-apps-openstack app.

In order to accomplished that, the below flags have been introduced:

--cluster-topology, set to false by default, when used cluster will be templated as App CR,
--cluster-user-configmap, the current command supports multiple flags for customizing cluster templating, though in the case of App CRs, the configuration must be provided in a form of values.yaml. We could try to translate the existing cluster flags to values.yaml before running the template app commands, and in fact I tried that at the beginning, though it quickly started to look messy and felt counterintuitive. Working with the values.yaml directly seems much more easier and flexible in configuration to the users, so I introduced this extra flag,
--default-apps-user-configmap, the same reasoning as above applies here,
--cluster-app-version, allows cluster app version selection,
--default-apps-app-version, allows default apps app version selection.

Examples

$ kubectl gs template cluster --provider openstack --cluster-topology --organization giantswarm-2

Output:

---
apiVersion: application.giantswarm.io/v1alpha1
kind: App
metadata:
  labels:
    app-operator.giantswarm.io/version: 0.0.0
  name: ox0f8-cluster
  namespace: org-giantswarm-2
spec:
  catalog: control-plane-catalog
  config:
    configMap:
      name: ""
      namespace: ""
    secret:
      name: ""
      namespace: ""
  kubeConfig:
    context:
      name: ""
    inCluster: true
    secret:
      name: ""
      namespace: ""
  name: cluster-openstack
  namespace: org-giantswarm-2
  version: 0.1.0
---
apiVersion: application.giantswarm.io/v1alpha1
kind: App
metadata:
  labels:
    app-operator.giantswarm.io/version: 0.0.0
  name: ox0f8-default-apps
  namespace: org-giantswarm-2
spec:
  catalog: default
  config:
    configMap:
      name: ""
      namespace: ""
    secret:
      name: ""
      namespace: ""
  kubeConfig:
    context:
      name: ""
    inCluster: true
    secret:
      name: ""
      namespace: ""
  name: default-apps-openstack
  namespace: org-giantswarm-2
  version: 0.1.0

$ kubectl gs template cluster --provider openstack --cluster-topology --organization giantswarm-2 --cluster-user-configmap /tmp/cm.yaml

---
apiVersion: v1
data:
  values: |
    clusterDescription: "My test cluster"
    organization: "giantswarm-2"
    cloudConfig: cloud-config
    cloudName: openstack

    kubernetesVersion: 1.20.9
    releaseVersion: 20.0.0-alpha1

    nodeClasses:
    - name: small
      machineFlavor: n1.small
      diskSize: 20

    controlPlane:
      class: default
      replicas: 3

    nodePools:
    - name: default
      class: default
      replicas: 3
kind: ConfigMap
metadata:
  creationTimestamp: null
  name: zi95g-cluster-userconfig
  namespace: org-giantswarm-2
---
apiVersion: application.giantswarm.io/v1alpha1
kind: App
metadata:
  labels:
    app-operator.giantswarm.io/version: 0.0.0
  name: zi95g-cluster
  namespace: org-giantswarm-2
spec:
  catalog: control-plane-catalog
  config:
    configMap:
      name: ""
      namespace: ""
    secret:
      name: ""
      namespace: ""
  kubeConfig:
    context:
      name: ""
    inCluster: true
    secret:
      name: ""
      namespace: ""
  name: cluster-openstack
  namespace: org-giantswarm-2
  userConfig:
    configMap:
      name: zi95g-cluster-userconfig
      namespace: org-giantswarm-2
  version: 0.1.0
---
apiVersion: application.giantswarm.io/v1alpha1
kind: App
metadata:
  labels:
    app-operator.giantswarm.io/version: 0.0.0
  name: zi95g-default-apps
  namespace: org-giantswarm-2
spec:
  catalog: default
  config:
    configMap:
      name: ""
      namespace: ""
    secret:
      name: ""
      namespace: ""
  kubeConfig:
    context:
      name: ""
    inCluster: true
    secret:
      name: ""
      namespace: ""
  name: default-apps-openstack
  namespace: org-giantswarm-2
  version: 0.1.0

$ kubectl gs template cluster --provider openstack --organization giantswarm-2 --release 20.0.0-alpha1 --node-machine-flavor=n1.small (some output has been cut for brevity)

---
apiVersion: cluster.x-k8s.io/v1alpha4
kind: Cluster
metadata:
  annotations:
    cluster.giantswarm.io/description: ""
  labels:
    cluster.x-k8s.io/cluster-name: kz4j9
    giantswarm.io/cluster: kz4j9
    giantswarm.io/organization: giantswarm-2
    release.giantswarm.io/version: 20.0.0-alpha1
  name: kz4j9
  namespace: org-giantswarm-2
spec:
  controlPlaneEndpoint:
    host: ""
    port: 0
  controlPlaneRef:
    apiVersion: controlplane.cluster.x-k8s.io/v1alpha4
    kind: KubeadmControlPlane
    name: kz4j9
    namespace: org-giantswarm-2
  infrastructureRef:
    apiVersion: infrastructure.cluster.x-k8s.io/v1alpha4
    kind: OpenStackCluster
    name: kz4j9
    namespace: org-giantswarm-2
status:
  infrastructureReady: false
---
...
---
apiVersion: infrastructure.cluster.x-k8s.io/v1alpha4
kind: OpenStackMachineTemplate
metadata:
  labels:
    cluster.x-k8s.io/cluster-name: kz4j9
    giantswarm.io/cluster: kz4j9
    giantswarm.io/organization: giantswarm-2
    release.giantswarm.io/version: 20.0.0-alpha1
  name: kz4j9
  namespace: org-giantswarm-2
spec:
  template:
    spec:
      cloudName: openstack
      flavor: n1.small
      identityRef:
        kind: Secret
        name: cloud-config
      image: ubuntu-2004-kube-v1.20.9
      rootVolume: {}

cmd/template/cluster/provider/capo.go

cmd/template/app/runner.go

cmd/template/cluster/flag.go

cmd/template/cluster/provider/capo.go

cmd/template/cluster/provider/common.go

cmd/template/cluster/runner.go

cmd/template/cluster/provider/capo.go

rossf7

@ljakimczuk This is looking good but CI isn't happy. Can you check and appease the Circle gods? :)

ljakimczuk · 2022-01-18T14:02:16Z

This is looking good but CI isn't happy. Can you check and appease the Circle gods? :)

Sure!

rossf7

Nice<3 this is looking good. Just some final nits on ordering.

We should get a review from Sig UX before merging but we can start with a review from the team.

We'll also need a docs PR 📚 https://docs.giantswarm.io/ui-api/kubectl-gs/template-cluster/

cmd/template/cluster/flag.go

ljakimczuk · 2022-01-18T15:52:52Z

We'll also need a docs PR.

@rossf7 I remember about that, but let's maybe wait for that until we get more reviews, just in case we have more changes coming in. WDYT?

rossf7 · 2022-01-18T15:59:24Z

maybe wait for that until we get more reviews, just in case we have more changes coming in. WDYT?

@ljakimczuk Yes that's fine 👍

cmd/template/cluster/flag.go

anvddriesch

Cool stuff!
Please list all the changes to the CHANGELOG.

cmd/template/cluster/flag.go

rossf7

LGTM when @anvddriesch and team 🌈 are happy

Agree it makes sense to hide the openstack flags for now.

ljakimczuk · 2022-01-19T09:19:22Z

@rossf7 @anvddriesch @tfussell done, flags are now hidden and the changlog entry is there, let me know if I should make it more detailed.

anvddriesch

LGTM

kopiczko · 2022-01-19T11:03:57Z

Do we need to keep raw at all?

ljakimczuk · 2022-01-19T11:34:01Z

@kopiczko not necessarily, it seemed more consistent to me to have this type, but it would work just as good without it. We can get rid of it or change this flag to something not suggesting multiple types, for example, --as-app or similar

kopiczko · 2022-01-19T11:52:37Z

can get rid of it or change this flag to something not suggesting multiple types, for example, --as-app or similar

I thought we could replace existing templates and ditch them altogether.

ljakimczuk · 2022-01-19T12:07:07Z

I believe the original idea was to support both templates types, at least for some time since some users may be interested in templating raw resources, but maybe I got it wrong. @rossf7 WDYT?

kopiczko · 2022-01-19T12:27:44Z

Asked @cornelius-keller

cornelius-keller · 2022-01-19T12:55:15Z

I think currently it does not hurt and gives the customer the opportunity to create the clusters with the old method still, which might be useful in case there are problems with the new ones and we need to compare them to see what is different.

In the long run I agree with @kopiczko and it should disappear or be renamed, especially because customers can expect that the raw method templates the same cluster as the app method only that it is the raw template and not wrapped into an app.

Maybe it will be useful to provide an option to get a raw template that is the same as included in the app, but that is a different story.

LGTM

kopiczko · 2022-01-19T12:57:55Z

It would be nice to at least generate raw templates using helm template to make sure they are always on par.

ljakimczuk · 2022-01-19T15:08:43Z

It would be nice to at least generate raw templates using helm template to make sure they are always on par.

This may be misleading for those relying on the "old" version for the migration time being and as far as I understood that's indeed the case. But since app is mostly about adding ClusterTopology, @tfussell suggested we could change the flag to --cluster-topology [true|false], @tfussell would you like to add something to that?

cmd/template/cluster/provider/capo.go

marians · 2022-01-19T15:52:22Z

Haven't read through the entire discussion.

One minor thing that stands out to me is the duplication of the term template in the flag --template-type. If we need that flag, could it simply be --type ?

ljakimczuk · 2022-01-19T16:38:38Z

One minor thing that stands out to me is the duplication of the term template in the flag --template-type. If we need that flag, could it simply be --type

I duplicated term template to emphasize the fact it's about the template type. I was afraid purpose of the --type alone may be vague having all the providers and different resources generated.

@tfussell has already proposed to change this flag to something else, namely --cluster-topology [true|false], that more closely describe the purpose of having an app for cluster, what do you think @marians ?

When we have an agreement I will push the changes.

marians · 2022-01-19T20:11:31Z

With a boolean flag, the default (when the flag is not used) should always be false. So --cluster-topology false would be the default. I'd like to make sure that the default makes sense.

What sort of output will that (--cluster-topology false) generate then, and what will --cluster-topology or --cluster-topology true generate instead?

ljakimczuk · 2022-01-20T10:32:04Z

@marians The --cluster-topology false in this scenario would behave the same way the --template-type raw (or lack of it) behaves. The cluster-topology true would behave the same way the --template-type app behaves right now.

MarcelMue reviewed Jan 17, 2022

View reviewed changes

cmd/template/cluster/provider/capo.go Outdated Show resolved Hide resolved

rossf7 reviewed Jan 18, 2022

View reviewed changes

cmd/template/app/runner.go Outdated Show resolved Hide resolved

cmd/template/cluster/flag.go Outdated Show resolved Hide resolved

cmd/template/cluster/flag.go Outdated Show resolved Hide resolved

cmd/template/cluster/flag.go Outdated Show resolved Hide resolved

ljakimczuk requested a review from rossf7 January 18, 2022 12:15

rossf7 reviewed Jan 18, 2022

View reviewed changes

ljakimczuk requested a review from a team January 18, 2022 14:49

ljakimczuk changed the title ~~[WIP] Templating Cluster as App CRs~~ Templating Openstack Clusters as App CRs Jan 18, 2022

ljakimczuk changed the title ~~Templating Openstack Clusters as App CRs~~ Support templating Openstack clusters as App CRs Jan 18, 2022

rossf7 reviewed Jan 18, 2022

View reviewed changes

cmd/template/cluster/flag.go Show resolved Hide resolved

ljakimczuk marked this pull request as ready for review January 18, 2022 15:50

ljakimczuk requested a review from a team January 18, 2022 15:50

ljakimczuk requested a review from a team January 18, 2022 15:56

rossf7 requested a review from tfussell January 18, 2022 15:59

tfussell reviewed Jan 18, 2022

View reviewed changes

cmd/template/cluster/flag.go Outdated Show resolved Hide resolved

tfussell approved these changes Jan 18, 2022

View reviewed changes

anvddriesch reviewed Jan 19, 2022

View reviewed changes

cmd/template/cluster/flag.go Outdated Show resolved Hide resolved

rossf7 approved these changes Jan 19, 2022

View reviewed changes

anvddriesch approved these changes Jan 19, 2022

View reviewed changes

rossf7 reviewed Jan 19, 2022

View reviewed changes

cmd/template/cluster/provider/capo.go Outdated Show resolved Hide resolved

ljakimczuk requested a review from a team January 20, 2022 13:56

ljakimczuk added 12 commits January 20, 2022 17:06

Playing with Cluster App templating

6b0d543

Templating App clusters

ba3842c

Exporting template app

8f9b667

Exporting rest of the flags for consistency

c95bca6

Reorganizing to not export runners from cmd/template/app

096cc4e

Updating after review

8716a86

Fixing imports

a2d5466

change ordering of flags

0fe4687

Hidding flags

6d25568

Adding entry to Changlog

ec2f48b

Removing suffix

f42f8c3

Changing flag to the cluster-topology

80884bb

ljakimczuk force-pushed the feature/cluster-app branch from 56b61a1 to 80884bb Compare January 20, 2022 16:07

marians approved these changes Jan 20, 2022

View reviewed changes

ljakimczuk merged commit b37a5db into master Jan 20, 2022

ljakimczuk deleted the feature/cluster-app branch January 20, 2022 16:33

This was referenced Jan 21, 2022

Enable cluster topology for OpenStack by default and update cluster-openstack default version to 0.3.0 #638

Merged

Allow configuration of cluster-openstack and default-apps-openstack catalogs #640

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support templating Openstack clusters as App CRs #635

Support templating Openstack clusters as App CRs #635

ljakimczuk commented Jan 17, 2022 •

edited

Loading

rossf7 left a comment

ljakimczuk commented Jan 18, 2022

rossf7 left a comment

ljakimczuk commented Jan 18, 2022 •

edited

Loading

rossf7 commented Jan 18, 2022

anvddriesch left a comment

rossf7 left a comment

ljakimczuk commented Jan 19, 2022

anvddriesch left a comment

kopiczko commented Jan 19, 2022

ljakimczuk commented Jan 19, 2022

kopiczko commented Jan 19, 2022

ljakimczuk commented Jan 19, 2022

kopiczko commented Jan 19, 2022

cornelius-keller commented Jan 19, 2022

kopiczko commented Jan 19, 2022

ljakimczuk commented Jan 19, 2022

marians commented Jan 19, 2022

ljakimczuk commented Jan 19, 2022

marians commented Jan 19, 2022

ljakimczuk commented Jan 20, 2022

Support templating Openstack clusters as App CRs #635

Support templating Openstack clusters as App CRs #635

Conversation

ljakimczuk commented Jan 17, 2022 • edited Loading

Description

Examples

rossf7 left a comment

Choose a reason for hiding this comment

ljakimczuk commented Jan 18, 2022

rossf7 left a comment

Choose a reason for hiding this comment

ljakimczuk commented Jan 18, 2022 • edited Loading

rossf7 commented Jan 18, 2022

anvddriesch left a comment

Choose a reason for hiding this comment

rossf7 left a comment

Choose a reason for hiding this comment

ljakimczuk commented Jan 19, 2022

anvddriesch left a comment

Choose a reason for hiding this comment

kopiczko commented Jan 19, 2022

ljakimczuk commented Jan 19, 2022

kopiczko commented Jan 19, 2022

ljakimczuk commented Jan 19, 2022

kopiczko commented Jan 19, 2022

cornelius-keller commented Jan 19, 2022

kopiczko commented Jan 19, 2022

ljakimczuk commented Jan 19, 2022

marians commented Jan 19, 2022

ljakimczuk commented Jan 19, 2022

marians commented Jan 19, 2022

ljakimczuk commented Jan 20, 2022

ljakimczuk commented Jan 17, 2022 •

edited

Loading

ljakimczuk commented Jan 18, 2022 •

edited

Loading