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

Feature/manifest as crd #872

Merged
merged 33 commits into from
Jun 20, 2023
Merged

Feature/manifest as crd #872

merged 33 commits into from
Jun 20, 2023

Conversation

cloudziu
Copy link
Contributor

This PR implements partially #357 and resolves #787.
The Frontend service has been redone to instead of using secret files as Claudie manifests, now it use CRD.

group: claudie.io
version: v1beta1
kind: inputmanifest

List of changes that will change UX:

  • Secret data has been offloaded from the manifest: Now the inputManifest only reference a secret resource in the provider sections.
  • The secret placement isn't limited upfront by namespace, it can be created in any namespace. The same goes for InputManifest. There is no need for any additional labels on the InputManifest/Secret resource
  • Because of the architecture, the validation webhook won't check the provider's section at kubectl apply level. The workflow will be similar to Kubernetes Pods scenario. If a Pod referencing a non-existing secret will be created - it will CrashLoop in an error state, Claudie will generate an error and check again in 60s. (Interval of REQUEUE_AFTER_ERROR const var).
  • When deleting an InputManifest - the file won't get removed until the cluster won't be removed, because of v1beta1.claudie.io/finalizer read more
# Create a secret
apiVersion: v1
kind: Secret
metadata:
  name: hetzner-1
  namespace: any-namespace
type: Opaque
data:
  apitoken: QSBUWSBDV0FOSUFDWktVCg==
 ---
 # Create an InputManifest
apiVersion: claudie.io/v1beta1
kind: InputManifest
metadata:
  name: hetzner-test-manifest
  namespace: claudie
spec:
  providers:
      - name: hetzner-secret
        providerType: hetzner
        secretRef:
          name: hetzner-1
          namespace: any-namespace 
  nodePools:
    dynamic:
      - name: control-hetzner-1
        providerSpec:
          name: hetzner-secret
          region: nbg1
          zone: nbg1-dc3
        count: 1
        serverType: cpx11
        image: ubuntu-22.04

    . . . 

  kubernetes:
    clusters:
      - name: hetzner-test-cluster
        version: v1.30.0
        network: 192.168.2.0/24
        pools:
          control:
            - control-hetzner-1
          compute:
            - compute-hetzner-1

ToDo:

  • Alter test sets, so they will utilise inputmanifest type.

@MarioUhrik
Copy link
Contributor

Because of the architecture, the validation webhook won't check the provider's section at kubectl apply level. The workflow will be similar to Kubernetes Pods scenario. If a Pod referencing a non-existing secret will be created - it will CrashLoop in an error state, Claudie will generate an error and check again in 60s. (Interval of REQUEUE_AFTER_ERROR const var).

I don't like this much, but I understand this is the best way.
I wouldn't want the creation of a CRD to be blocked by the absence of the secret.
However, I think the default interval should be much shorter, no more than 5 seconds.

@MarioUhrik
Copy link
Contributor

Will the status of the inputManifest be updated by events and errors?

Copy link
Contributor

@Despire Despire left a comment

Choose a reason for hiding this comment

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

Nice work thus far 👍

services/frontend/main.go Outdated Show resolved Hide resolved
services/frontend/pkg/controller/validator.go Outdated Show resolved Hide resolved
services/frontend/pkg/controller/validator.go Outdated Show resolved Hide resolved
services/frontend/pkg/controller/controller_types.go Outdated Show resolved Hide resolved
services/frontend/pkg/controller/controller.go Outdated Show resolved Hide resolved
…t type, Target PR comments, Edit testing-framework that it adapts inputmanifest type
@cloudziu
Copy link
Contributor Author

cloudziu commented Jun 16, 2023

Will the status of the inputManifest be updated by events and errors?

Yes, the inputmanifest .status field will contain the status of the cluster (including errors) + when an error will occur an Kubernetes event will be created with the err message.

Copy link
Contributor

@MiroslavRepka MiroslavRepka left a comment

Choose a reason for hiding this comment

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

Great work @cloudziu!

manifests/testing-framework/README.md Outdated Show resolved Hide resolved
manifests/testing-framework/README.md Outdated Show resolved Hide resolved
services/frontend/pkg/api/v1beta1/inputmanifest_helpers.go Outdated Show resolved Hide resolved
@cloudziu cloudziu marked this pull request as ready for review June 19, 2023 08:40
@cloudziu cloudziu added the Disable clean up This label is used when E2E tests should not delete infra after fail label Jun 20, 2023
@cloudziu cloudziu requested a review from Despire June 20, 2023 13:07
@cloudziu cloudziu added this pull request to the merge queue Jun 20, 2023
Merged via the queue into master with commit a824055 Jun 20, 2023
1 check passed
@cloudziu cloudziu deleted the feature/manifest-as-crd branch June 20, 2023 13:18
@cloudziu cloudziu mentioned this pull request Jun 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Disable clean up This label is used when E2E tests should not delete infra after fail
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature: offload provider secrets from input manifest
4 participants