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

fix(kafka): add clusterName because of api-changes #1624

Merged
merged 1 commit into from
Jan 12, 2023

Conversation

haarchri
Copy link
Member

@haarchri haarchri commented Jan 8, 2023

Signed-off-by: Christopher Paul Haar [email protected]

Description of your changes

fix(kafka): add clusterName because of api-changes

Fixes #1612
replace: #1620

I have:

  • Read and followed Crossplane's contribution process.
  • Run make reviewable test to ensure this PR is ready for review.

How has this code been tested

kubectl get managed

NAME                                      READY   SYNCED   EXTERNAL-NAME
cluster.kafka.aws.crossplane.io/example   True   True     arn:aws:kafka:us-east-1:12345678910:cluster/example/cef294f9-8e22-4342-9bfd-9635fcaeaebb-9

NAME                                                          READY   SYNCED   EXTERNAL-NAME
configuration.kafka.aws.crossplane.io/example-configuration   True    True     arn:aws:kafka:us-east-1:12345678910:configuration/example-configuration/73a88bf4-8aee-485e-b004-a788735643d4-9

@@ -479,6 +482,7 @@ spec:
description: Create tags when creating the cluster.
type: object
required:
- clusterName
Copy link
Collaborator

Choose a reason for hiding this comment

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

would defaulting this parameter to the object name provide backwards compatibility? Would it make sense?

Copy link
Member Author

Choose a reason for hiding this comment

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

problem is for people using compositions - the cr.name is complete different

@haarchri haarchri merged commit cbc5c75 into crossplane-contrib:master Jan 12, 2023
@abrarcci
Copy link

FYI - have tested out the new changes @haarchri and the msk cluster is now being provisioned, thank you!

One point to note, doing a describe on the cluster gives the following:

  Warning  CannotObserveExternalResource  3m22s (x60 over 58m)  managed/cluster.kafka.aws.crossplane.io  failed to describe Cluster: InvalidParameter: 1 validation error(s) found.
- missing required field, DescribeClusterInput.ClusterArn.

And the status of the cluster never changes from this:

Status:
  At Provider:
  Conditions:
    Last Transition Time:  2023-01-17T12:50:14Z
    Reason:                Creating
    Status:                False
    Type:                  Ready
    Last Transition Time:  2023-01-17T12:50:14Z
    Message:               observe failed: failed to describe Cluster: InvalidParameter: 1 validation error(s) found.
- missing required field, DescribeClusterInput.ClusterArn.

@haarchri
Copy link
Member Author

Yes we found the same we will update shortly

@Patrick-L Patrick-L mentioned this pull request Jan 18, 2023
2 tasks
@haarchri
Copy link
Member Author

we will publish tomorrow a new fix release #1638

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants