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 helper functions for ipcidr #116

Merged
merged 2 commits into from
Sep 9, 2024
Merged

Conversation

Issacwww
Copy link
Contributor

Description of changes:
Add new helper function for EKS to config the template
have to reopen new PR due to #102 uses develop branch and need to discard original commit to sync with latest.

Testing done:
Added unit test and cargo test passed

Terms of contribution:

By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.

Copy link
Contributor

@cbgbt cbgbt left a comment

Choose a reason for hiding this comment

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

This looks great! There are only a few small changes I'd request.

Seems like the CI is failing due to unformatted sources. Mind running cargo fmt on schnauzer?

You can check run all of the CI lints, etc, ahead of time by running make twoliter check.

Comment on lines 45 to 46
settings-extension-oci-defaults.workspace = true
cidr = "0.2.3"
Copy link
Contributor

@cbgbt cbgbt Aug 30, 2024

Choose a reason for hiding this comment

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

The CI should catch this when it runs, but you'll want to use workspace dependencies for this. Basically you'll want to:

  • Add cidr = 0.2 to the [workspace.dependencies] list in sources/Cargo.toml
  • Change this to cidr.workspace = true.

make twoliter check should catch this too, if you want to try and replicate locally.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for sharing this!
encountered in local run. Any clue what might be missed? I simply run the make twoliter check and testing looks good

[cargo-make] INFO - Running Task: check-migrations
grep: Release.toml: No such file or directory
[cargo-make] ERROR - Error while executing command, exit code: 2
[cargo-make] WARN - Build Failed.
Error: Command was unsuccessful, exit code 1
make: *** [twoliter] Error 1

Copy link
Contributor

Choose a reason for hiding this comment

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

This actually seems to be a bug twoliter, sorry about that. I opened bottlerocket-os/twoliter#359 to track it.

We'll provide a fix for that, but in the meantime you'll have to separately run the individual check tasks: make twoliter unit-tests check-fmt check-lintsl.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, the individual check looks good to me
[cargo-make] INFO - Build Done in 52.05 seconds.

sources/api/schnauzer/src/helpers/mod.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@cbgbt cbgbt left a comment

Choose a reason for hiding this comment

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

Looks great! 🚀

Copy link
Contributor

@yeazelm yeazelm left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for the contribution!

@yeazelm yeazelm merged commit adc18ca into bottlerocket-os:develop Sep 9, 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