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

addon: seems to be missing 'addon name' field #149

Closed
natereid72 opened this issue Nov 15, 2022 · 13 comments
Closed

addon: seems to be missing 'addon name' field #149

natereid72 opened this issue Nov 15, 2022 · 13 comments
Labels
bug Something isn't working needs:triage-stale

Comments

@natereid72
Copy link

What happened?

The addon resource appears to rely on metadata.name to provide the addon name to aws api. This will work if a managed resource is created manually and no other additional instance of the addon needs to exist in the cluster. But if you need to create multiple addons of the same type or are using Composition that wants to make metadata.name unique, this is a blocker.

How can we reproduce it?

Create a addon via composition and see the error.

    - name: cluster-csi-driver
      base:
        apiVersion: eks.aws.upbound.io/v1beta1
        kind: Addon
        metadata:
          name: aws-ebs-csi-driver
        spec:
          forProvider:
            addonVersion: "v1.11.4-eksbuild.1"
            clusterNameSelector:
              matchControllerRef: true
            region: us-west-2
            serviceAccountRoleArnSelector:
              matchControllerRef: true
              matchLabels:
                role: nodegroup
Message:               apply failed: error creating EKS Add-On (nate-demo-again-mksgm-t459x:nate-demo-again-mksgm-k8rtp): InvalidParameterException: Addon specified is not supported

What environment did it happen in?

  • Universal Crossplane Version: 1.10.x
  • Provider Version: 0.20.0
  • EKS v1.23 cluster
@natereid72 natereid72 added the bug Something isn't working label Nov 15, 2022
@ulucinar
Copy link
Collaborator

ulucinar commented Nov 16, 2022

Hi @natereid72,
Looks like what happens is Crossplane overrides the metadata.name that the composed template specifies in the Composition. But my assumption is that you need to choose an addon name from a predefined set by EKS.

I would suggest to utilize the external-name annotation in this case. Could you please give a try to something like:

- name: cluster-csi-driver
      base:
        apiVersion: eks.aws.upbound.io/v1beta1
        kind: Addon
        metadata:
          annotations:
            crossplane.io/external-name: aws-ebs-csi-driver
        spec:
...

@muvaf
Copy link
Member

muvaf commented Nov 16, 2022

@ulucinar I think this is one of the cases where it should be left as spec field instead of using external name since metadata.name would never be the right value.

@natereid72
Copy link
Author

my suggestion is:

spec:
  forProvider:
    addon:
      name:
      version:

@ulucinar
Copy link
Collaborator

We had an offline chat with @natereid72, and I will provide more context in this issue discussing some possibilities.

@adrienzieba
Copy link

Hello,

I'm experiencing something similar. In my composition the name of the resource should be unique and with the current CRD it is not something I can do.
Is there any update on that?

Thanks

@bhavinkotak
Copy link

@muvaf any workaround for this issue? Facing similar issue for other resources as well which are currently relying on metadata.name. For e.g., cloudwatchlogs.Group

@bhavinkotak
Copy link

@natereid72 - any plan of getting this issue resolved in upcoming releases?

@natereid72
Copy link
Author

@bhavinkotak , you'd have to check with someone else. I don't have any involvement in the project planning. Last I knew, it was a bit of 'painted into a corner' issue. Not implemented in the best way, but implemented. Which will result in a breaking change to the provider as a CRD needs to be modified.

@ulucinar
Copy link
Collaborator

ulucinar commented Feb 6, 2023

Hi @bhavinkotak,
You can make use of the external-name annotation as a remedy:

apiVersion: eks.aws.upbound.io/v1beta1
kind: Addon
metadata:
  annotations:
    crossplane.io/external-name: vpc-cni
  name: addon1
spec:
  forProvider:
    clusterNameSelector:
      matchLabels:
        testing.upbound.io/example-name: cluster1
    region: us-west-1

---

apiVersion: eks.aws.upbound.io/v1beta1
kind: Addon
metadata:
  annotations:
    crossplane.io/external-name: vpc-cni
  name: addon2
spec:
  forProvider:
    clusterNameSelector:
      matchLabels:
        testing.upbound.io/example-name: cluster2
    region: us-west-1

So the above two resources have different metadata.names, whereas the same crossplane.io/external-name annotations. They both install the vpc-cni addon to two different clusters.

Could you please give this a try?

@adamhouse
Copy link

@ulucinar I have a semi-related question about this. I've defined an Addon in my Composition similar to what you provided above:

    - name: eks-addon-vpc-cni
      base:
        apiVersion: eks.aws.upbound.io/v1beta1
        kind: Addon
        metadata:
          annotations:
            crossplane.io/external-name: vpc-cni
        spec:
          deletionPolicy: Delete
          forProvider:
            addonVersion: "v1.12.1-eksbuild.2"
            clusterNameSelector:
              matchControllerRef: true
            resolveConflicts: "OVERWRITE"

I can confirm that the Addon was successfully installed on my target cluster but I noticed that the MR eventually registers as SYNCED = false:

NAME                 READY   SYNCED   EXTERNAL-NAME   AGE
ahouse-sx7ks-ld9gk   False   True     vpc-cni         26m

NAME                 READY   SYNCED   EXTERNAL-NAME   AGE
ahouse-sx7ks-ld9gk   True    True     vpc-cni         27m

NAME                 READY   SYNCED   EXTERNAL-NAME   AGE
ahouse-sx7ks-ld9gk   True    False    vpc-cni         30m

When I describe the MR, I see the following:

  Conditions:
    Last Transition Time:  2023-02-06T21:27:35Z
    Message:               observe failed: cannot run plan: plan failed: Instance cannot be destroyed: Resource aws_eks_addon.ahouse-sx7ks-ld9gk has lifecycle.prevent_destroy set, but the plan calls for this resource to be destroyed. To avoid this error and continue with the plan, either disable lifecycle.prevent_destroy or reduce the scope of the plan using the -target flag.
    Reason:                ReconcileError
    Status:                False
    Type:                  Synced
    Last Transition Time:  2023-02-06T21:27:14Z
    Reason:                Available
    Status:                True
    Type:                  Ready
    Last Transition Time:  2023-02-06T21:26:48Z
    Reason:                Finished
    Status:                True
    Type:                  AsyncOperation
    Last Transition Time:  2023-02-06T21:26:48Z
    Message:               apply failed: unexpected EKS Add-On (ahouse-sx7ks-hw64l:vpc-cni) state returned during creation: timeout while waiting for state to become 'ACTIVE' (last state: 'CREATING', timeout: 20m0s)
[WARNING] Running terraform apply again will remove the kubernetes add-on and attempt to create it again effectively purging previous add-on configuration:
    Reason:  ApplyFailure
    Status:  False
    Type:    LastAsyncOperation

Can you help me understand what I might be doing wrong here?

@rozcietrzewiacz
Copy link

@adamhouse Seems strange, but can you verify that vpc-cni addon is recognized by aws in your account? According to the terraform module documentation:

The name must match one of the names returned by describe-addon-versions.

Thus you should check with:

aws eks describe-addon-versions --query='addons[].addonName'

@rozcietrzewiacz
Copy link

FWIW, I came across the same (i.e. original) issue with aws-ebs-csi-driver while trying to get persistent volumes working on a vanilla cluster provisioned with upbound/platform-ref-aws. Applying the following fix (thanks @ulucinar for the tip!) moves the needle a bit, but is not sufficient due to additional policy that needs to be attached to the instance node role.

apiVersion: eks.aws.upbound.io/v1beta1
kind: Addon
metadata:
  annotations:
    crossplane.io/external-name: aws-ebs-csi-driver
  name: fix1--add-ebs-csi
spec:
  forProvider:
    clusterNameSelector:
      matchLabels:
        crossplane.io/claim-name: platform-ref-aws
    region: us-west-2

@haarchri
Copy link
Member

checkout the following reference implementation upbound/platform-ref-aws#97 - thanks for rasing the issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working needs:triage-stale
Projects
None yet
Development

No branches or pull requests

9 participants