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

Add Cluster resource template #3

Merged
merged 9 commits into from
Oct 19, 2023
Merged

Add Cluster resource template #3

merged 9 commits into from
Oct 19, 2023

Conversation

nprokopic
Copy link
Contributor

Note: WIP, initially based on Cluster resource from cluster-aws.

What does this PR do?

(Please set a descriptive PR title. Use this space for additional explanations.)

What is the effect of this change to users?

Cluster resource is deployed.

How does it look like?

apiVersion: cluster.x-k8s.io/v1beta1
kind: Cluster
metadata:
  annotations:
    cluster.giantswarm.io/description: "Awesome Giant Swarm cluster"
  labels:
    app: "cluster"
    app.kubernetes.io/managed-by: "Helm"
    app.kubernetes.io/version: "0.0.0-dev"
    application.giantswarm.io/team: "turtles"
    giantswarm.io/cluster: "awesome"
    giantswarm.io/organization: "giantswarm"
    giantswarm.io/service-priority: highest
    cluster.x-k8s.io/cluster-name: "awesome"
    cluster.x-k8s.io/watch-filter: capi
    helm.sh/chart: "cluster-0.0.0-dev"
    cluster-apps-operator.giantswarm.io/watching: ""
  name: awesome
  namespace: default
spec:
  clusterNetwork:
    services:
      cidrBlocks:
        - 172.31.0.0/16
    pods:
      cidrBlocks:
        - 100.64.0.0/12
  controlPlaneRef:
    apiVersion: controlplane.cluster.x-k8s.io/v1beta1
    kind: KubeadmControlPlane
    name: awesome
  infrastructureRef:
    apiVersion: infrastructure.cluster.x-k8s.io/v1beta1
    kind: AWSCluster
    name: awesome

Any background context you can provide?

Working towards a PoC for restructured cluster apps (more details here).

What is needed from the reviewers?

PTAL :)

Do the docs need to be updated?

Not yet.

Should this change be mentioned in the release notes?

  • CHANGELOG.md has been updated (if it exists)

@nprokopic nprokopic force-pushed the add-cluster-resource branch from 8b30ba6 to 5c5b43d Compare July 4, 2023 10:29
Base automatically changed from add-helm-chart to main July 4, 2023 12:08
@nprokopic nprokopic force-pushed the add-cluster-resource branch from 359bb49 to a82e315 Compare October 18, 2023 10:47
@nprokopic nprokopic marked this pull request as ready for review October 18, 2023 13:48
@nprokopic nprokopic requested a review from a team as a code owner October 18, 2023 13:48
@nprokopic
Copy link
Contributor Author

It's expected that JSON schema validation is failing here, because the current validation has rules for existing cluster apps, and those rules do not work in the cluster chart.

@njuettner
Copy link
Member

I know this PR is only taking the source from https://github.com/giantswarm/cluster-aws/blob/master/helm/cluster-aws/templates/_cluster.tpl. Feel free to merge 👍🏻

services:
cidrBlocks:
- 172.31.0.0/16
internal:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We would statically set these from the cluster-$provider app in its Helm values like this:

cluster:
  internal:
    ...

and these would be treated as an internal API, as in internal in our apps, where the cluster chart exposes it to the cluster-$provider , but it's not meant to be set by end users. This is roughly aligned with the current behavior of Values.internal.

@nprokopic
Copy link
Contributor Author

I know this PR is only taking the source from...

In a nutshell all initial PRs will be like that, with "just" make the templates being nicely configurable, so all providers can use them, so we have more work around Helm values and JSON schema in order to make this work nicely.

@nprokopic nprokopic merged commit 1400a74 into main Oct 19, 2023
@nprokopic nprokopic deleted the add-cluster-resource branch October 19, 2023 11:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants