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

Package Kubenix script #62

Merged
merged 4 commits into from
May 6, 2024
Merged

Package Kubenix script #62

merged 4 commits into from
May 6, 2024

Conversation

pizzapim
Copy link
Contributor

@pizzapim pizzapim commented May 5, 2024

As suggested in #60, I used writeScriptBin and some other helpers to package the Kubenix script. I extracted the Bash script itself and put it in its own file which makes it easier to maintain. Using wrapProgram, I set the environmental variables that were before set by string substitution.

closes #59

@pizzapim
Copy link
Contributor Author

pizzapim commented May 5, 2024

BTW, this doesn't link the resulting Kubernetes manifest to the derivation output anymore because there is no need. I guess this could break someone's setup, but I believe they shouldn't be using the manifest generated for the Kubenix script.

@hall
Copy link
Owner

hall commented May 5, 2024

Thanks! This does look a lot neater 💯

@hall
Copy link
Owner

hall commented May 5, 2024

BTW, this doesn't link the resulting Kubernetes manifest to the derivation output anymore because there is no need. I guess this could break someone's setup, but I believe they shouldn't be using the manifest generated for the Kubenix script.

I think this is fine to do given the front page disclaimer about being unstable. I half-implemented a CHANGELOG.md which should tag the commit whenever a new version is added. That might be a good place to capture this potentially-breaking change if you'd like.

@pizzapim
Copy link
Contributor Author

pizzapim commented May 5, 2024

Does this look fine?

@adrian-gierakowski
Copy link
Contributor

adrian-gierakowski commented May 5, 2024

Haven’t looked at the changes, but just wanted to make sure that users will still be able to easily access the generated manifest. Not everyone deploys via kubectl apply from their CI\CD pipeline. For example where I work, we generate manifests in the CI\CD pipeline of the source repo and push them to a manifest repo. The manifest changes are then picked up and deployed by argocd running within the cluster.

@pizzapim
Copy link
Contributor Author

pizzapim commented May 5, 2024

The example to generate the manifest on the documentation website is still working fine (i.e. using kubenix.evalModules and config.kubernetes.result).

pkgs/kubenix.nix Outdated
{
kubeconfig = kubernetes.kubeconfig or "";

kubeconfig = "/home/pim/.kube/config"; # kubernetes.kubeconfig or "";
Copy link
Contributor

Choose a reason for hiding this comment

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

Forgot to remove personal path?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch, corrected

Copy link
Contributor

@adrian-gierakowski adrian-gierakowski left a comment

Choose a reason for hiding this comment

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

Maybe we could add the result to passthru on the derivation as passthru.manifest?

@pizzapim
Copy link
Contributor Author

pizzapim commented May 5, 2024

Seems fair, I've added it!

Copy link
Contributor

@adrian-gierakowski adrian-gierakowski left a comment

Choose a reason for hiding this comment

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

Looks good to me, for what it’s worth (I don’t have merge permissions 😄)

@hall hall merged commit c00c78b into hall:main May 6, 2024
1 check 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.

kubenix script has no shebang, leading to "Exec format error"
3 participants