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

Create containerattached Cluster resource #485

Merged
merged 3 commits into from
Mar 19, 2024

Conversation

RedbackThomson
Copy link
Contributor

@RedbackThomson RedbackThomson commented Mar 14, 2024

Description of your changes

Adds the google_container_attached_cluster resource to a newly created containerattached group

I have:

  • Read and followed Crossplane's contribution process.
  • Run make reviewable to ensure this PR is ready for review.
  • Added backport release-x.y labels to auto-backport this PR if necessary.

How has this code been tested

I was able to e2e test this by creating an EKS cluster with an OIDC issuer (using the XEKS composition from configuration-aws-eks). I copied the output OIDC Issuer URL into oidcConfig.issuerUrl, set the platformVersion to the newest version supported for the 1.28 Kubernetes cluster, and updated the distribution to eks.

Here is a redacted version of the manifest I applied:

apiVersion: containerattached.gcp.upbound.io/v1beta1
kind: Cluster
metadata:
  labels:
    testing.upbound.io/example-name: primary
  name: primary
spec:
  deletionPolicy: Delete
  forProvider:
    authorization:
    - adminGroups:
      - <redacted>
    description: Test cluster
    distribution: eks
    fleet:
    - project: projects/<redacted>
    location: us-west1
    loggingConfig:
    - componentConfig:
      - enableComponents:
        - SYSTEM_COMPONENTS
        - WORKLOADS
    monitoringConfig:
    - managedPrometheusConfig:
      - enabled: true
    oidcConfig:
    - issuerUrl: <redacted>
    platformVersion: 1.28.0-gke.3
    project: <redacted>

@jeanduplessis
Copy link
Collaborator

/test-examples="examples/containerattached/v1beta1/cluster.yaml"

@turkenf
Copy link
Collaborator

turkenf commented Mar 16, 2024

Hi @RedbackThomson,

Thank you for your effort to add this resource. We currently do not have a mechanism to automatically test this resource. Therefore, we need to test manually and add manual intervention annotation to the resource YAML file.

Let me explain the manual test steps:

  • As you mentioned, we need EKS or AKS cluster for the spec.forProvider.oidcConfig.issuerUrl parameter of the Cluster.containerattached resource. I did the test using AKS, but it is similar for EKS.
    • Install family provider provider-azure-containerservice
    • Create the resource KubernetesCluster.containerservice by adding the parameter oidcIssuerEnabled: true
    • Once the resource is created, save the status.atProvider.oidcIssuerUrl value and fill in the related parameter.
  • Fill in the spec.forProvider.fleet.project parameter with "projects/<project_number>"
  • For the spec.forProvider.platformVersion parameter, I created and checked a data source with Terraform, but you can use the valid value platformVersion: "1.28.0-gke.3"

After providing the tricky parameters in this way, let me share with you an example with manual intervention below:

apiVersion: containerattached.gcp.upbound.io/v1beta1
kind: Cluster
metadata:
  annotations:
    meta.upbound.io/example-id: containerattached/v1beta1/cluster
    upjet.upbound.io/manual-intervention: "This resource requires a valid issuerUrl value from the AKS or EKS cluster."
  labels:
    testing.upbound.io/example-name: primary
  name: primary
spec:
  forProvider:
    description: Test cluster
    distribution: aks
    fleet:
    - project: projects/${data.google_project.project.number}
    location: us-west1
    oidcConfig:
    - issuerUrl: https://oidc.issuer.url
    platformVersion: ${data.google_container_attached_versions.versions.valid_versions[0]}
    project: ${data.google_project.project.project_id}

@RedbackThomson
Copy link
Contributor Author

Added manual intervention annotation to the example.

I was able to test the resource with an existing attached cluster, and saw that it was able to reconcile the status into the resource. It looks like it works 👍🏼

Copy link
Collaborator

@turkenf turkenf left a comment

Choose a reason for hiding this comment

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

Thank you @RedbackThomson, it seems that there are uncommitted changes after make generate, could you please run make generate and commit all changes?
Screenshot 2024-03-19 at 23 48 54

And it would be great if you could add how you tested it in the PR description.

@RedbackThomson
Copy link
Contributor Author

@turkenf Looks like it needed a rebase and regenerate. Fixed up that commit and force pushed.

Added my test methodology to the description, with an example manifest.

@turkenf turkenf merged commit 4333b23 into crossplane-contrib:main Mar 19, 2024
8 of 9 checks passed
@RedbackThomson RedbackThomson deleted the containerattached branch March 19, 2024 22:36
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.

3 participants