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

Initial configuration #1

Merged
merged 9 commits into from
Oct 15, 2024
Merged

Initial configuration #1

merged 9 commits into from
Oct 15, 2024

Conversation

kaessert
Copy link
Collaborator

@kaessert kaessert commented Sep 27, 2024

Initial configuration for aws-lb-controller

I have:

  • Read and followed Upbound'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, as appropriate.

How has this code been tested

make e2e and manual verification

@kaessert kaessert force-pushed the tk/initial-config branch 2 times, most recently from 027b102 to 0ae6a92 Compare October 4, 2024 08:35
@kaessert
Copy link
Collaborator Author

kaessert commented Oct 4, 2024

/test-examples

@kaessert kaessert marked this pull request as ready for review October 4, 2024 13:53
@kaessert
Copy link
Collaborator Author

kaessert commented Oct 4, 2024

/test-examples

3 similar comments
@kaessert
Copy link
Collaborator Author

kaessert commented Oct 4, 2024

/test-examples

@kaessert
Copy link
Collaborator Author

kaessert commented Oct 4, 2024

/test-examples

@kaessert
Copy link
Collaborator Author

kaessert commented Oct 4, 2024

/test-examples

@kaessert
Copy link
Collaborator Author

kaessert commented Oct 4, 2024

/test-examples

1 similar comment
@kaessert
Copy link
Collaborator Author

kaessert commented Oct 4, 2024

/test-examples

@kaessert
Copy link
Collaborator Author

kaessert commented Oct 6, 2024

/test-examples

@kaessert
Copy link
Collaborator Author

kaessert commented Oct 8, 2024

/test-examples

1 similar comment
@kaessert
Copy link
Collaborator Author

kaessert commented Oct 8, 2024

/test-examples

@kaessert
Copy link
Collaborator Author

kaessert commented Oct 8, 2024

/test-examples

@kaessert kaessert marked this pull request as draft October 8, 2024 17:21
@kaessert kaessert force-pushed the tk/initial-config branch 2 times, most recently from 4f43bde to e4e8ba6 Compare October 9, 2024 10:43
@kaessert kaessert marked this pull request as ready for review October 9, 2024 10:45
@kaessert
Copy link
Collaborator Author

kaessert commented Oct 9, 2024

/test-examples

1 similar comment
@kaessert
Copy link
Collaborator Author

kaessert commented Oct 9, 2024

/test-examples

@@ -0,0 +1,5 @@
#!/bin/sh
Copy link
Member

Choose a reason for hiding this comment

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

why we need this ? is foreground deletion not enough ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Mhm i don't like the fact that we have extra shell scripts around for this

Copy link
Member

Choose a reason for hiding this comment

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

wonder how this workes before in platform-ref-aws - pre 1.0 uptest ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Neither do i :) What i tried to is to extend uptest to manipulate the wait-parameter but even then it failed because even though it waited for the delete of the XR, the usage was still lingering around making it fail.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Damn, i tried it again with the older version but there the import step fails with some i think kuttl-related stuff and i don't see a way to skip the import in the older version
uptest-v0.13.0: error: unknown long flag '--skip-import'

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We tried several things, none of them seem reasonable at this point. Please check crossplane/uptest#33 for futher details.

@kaessert
Copy link
Collaborator Author

kaessert commented Oct 9, 2024

/test-examples

Copy link
Member

@ytsarev ytsarev left a comment

Choose a reason for hiding this comment

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

I left some comments for the first review round. I need to test it e2e on my side to provide deeper feedback.

Copy link
Member

@ytsarev ytsarev left a comment

Choose a reason for hiding this comment

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

2024/10/15 11:54:11         === STDOUT
2024/10/15 11:54:11         internetgateway.ec2.aws.upbound.io/configuration-aws-lb-controller-q459v condition met
2024/10/15 11:54:11         subnet.ec2.aws.upbound.io/configuration-aws-lb-controller-g48tp condition met
2024/10/15 11:54:11         subnet.ec2.aws.upbound.io/configuration-aws-lb-controller-zzzbh condition met
2024/10/15 11:54:11         vpc.ec2.aws.upbound.io/configuration-aws-lb-controller-vz9nd condition met
2024/10/15 11:54:11         cluster.eks.aws.upbound.io/configuration-aws-lb-controller-ctffw condition met
2024/10/15 11:54:11     | 11:54:11 | delete | Assert Deletion  | SCRIPT    | DONE  |
2024/10/15 11:54:11     | 11:54:11 | delete | Assert Deletion  | TRY       | DONE  |
2024/10/15 11:54:11 --- PASS: chainsaw (0.00s)
2024/10/15 11:54:11     --- PASS: chainsaw/delete (701.51s)
2024/10/15 11:54:11 PASS
2024/10/15 11:54:11 Tests Summary...
2024/10/15 11:54:11 - Passed test 1
2024/10/15 11:54:11 - Failed  tests 0
2024/10/15 11:54:11 - Skipped tests 0
2024/10/15 11:54:11 Done.
11:54:11 [ OK ] running automated tests

The test run is green, including clean deletion 👍

I left a couple of suggestions inline.

@kaessert kaessert force-pushed the tk/initial-config branch 2 times, most recently from 0006e00 to 5610ada Compare October 15, 2024 10:05
Copy link
Member

@ytsarev ytsarev left a comment

Choose a reason for hiding this comment

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

LGTM

@kaessert
Copy link
Collaborator Author

/test-examples

@kaessert kaessert merged commit f8dd9e0 into upbound:main Oct 15, 2024
2 checks passed
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