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

Add controller:v1.11.0 rockcraft specs and tests. #4

Merged
merged 31 commits into from
Aug 20, 2024

Conversation

aznashwan
Copy link
Contributor

No description provided.

Copy link
Member

@berkayoz berkayoz left a comment

Choose a reason for hiding this comment

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

Did a first pass, great work left some comments

controller/v1.10.1/rockcraft.yaml Outdated Show resolved Hide resolved
controller/v1.10.1/rockcraft.yaml Outdated Show resolved Hide resolved
controller/v1.11.0/rockcraft.yaml Outdated Show resolved Hide resolved
controller/v1.11.0/rockcraft.yaml Outdated Show resolved Hide resolved
kube-webhook-certgen/v1.4.0/nsswitch.conf Outdated Show resolved Hide resolved
@aznashwan aznashwan force-pushed the nginx-rocks branch 2 times, most recently from a50970c to edefd4c Compare August 14, 2024 14:42
Signed-off-by: Nashwan Azhari <[email protected]>
Signed-off-by: Nashwan Azhari <[email protected]>
Copy link
Collaborator

@bschimke95 bschimke95 left a comment

Choose a reason for hiding this comment

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

LGTM, only a couple of nits

# BusyBox included to facilitate debugging, which we will conditionally
# include based on this environment variable.
# https://github.com/GoogleContainerTools/distroless/blob/a019fc2/base/base.bzl#L157
# TODO(aznashwan): set this back to 0:
Copy link
Collaborator

Choose a reason for hiding this comment

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

set back to 0

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately can't since we need busybox (or some other combination of a shell + a handful of individual coreutils) to run the entrypoint.

This is due to the kube-webhook-certgen binary executing in less than 1 second, so Pebble does NOT heed the on-success: shutdown and just hangs indefinitely, even if the exit code is 0, hence the need for the hacky entrypoint script too too...

I will, however, be removing this conditional and updating the comment.

tests/integration/test_nginx_components_in_helm_chart.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@bschimke95 bschimke95 left a comment

Choose a reason for hiding this comment

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

LGTM

@bschimke95 bschimke95 merged commit 915243d into canonical:main Aug 20, 2024
5 of 13 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