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

[tenant-namespace-operator] Switched to helm module in k8s ansible collection #9

Merged
merged 2 commits into from
May 12, 2020

Conversation

plnordquist
Copy link

  • Updated to latest community.kubernetes collection
  • Switched to relative paths for watches.yaml for easier development
  • Added symlink for build/Dockerfile for operator-sdk run --local
  • Added collection references to role meta
  • Removed helm-diff plugin since helm module checks values and version for changes
  • Removed temp file in role as helm module accepts python dict
  • Switched to snake case reference to flavor
  • Added label for namespace type

As listed above I've symlinked the Dockerfile so the operator sdk can run locally for fast development of the Ansible roles. I think that symlink can go away when the build scripts for the container can point to a Dockerfile not in the current working directory. The build script might need a configurable location to look for the /.extrafingerprints file as well since a non-root user can't write to / without pre-creating the file as root.

I've updated the Dockerfile to drop root when running non-root required commands and skipped adding the miscscripts repository by passing the repository when calling helm pull. The unpacked chart now ends up in the home directory of the operator user which is easier to replicate in a local dev environment.

It looks like the version of the container is being put in the /.extrafingerprints file, should this PR update that version?

Updated to latest community.kubernetes collection
Switched to relative paths for watches.yaml for easier development
Added symlink for build/Dockerfile for operator-sdk run --local
Added collection references to role meta
Removed helm-diff plugin since helm module checks values and version for
changes
Removed temp file in role as helm module accepts python dict
Switched to snake case reference to flavor
Added label for namespace type
@kfox1111
Copy link

Looks good. Can you make a couple more tweaks please:
update containers/tenant-namespace-operator/buildenv to be export PREFIX=0.1.6
update charts/charts/tenant-namespace-operator/Chart.yaml and set version: 0.1.8 and appVersion: 0.1.6-1

Those are not automated yet unfortunately. Otherwise, looks great. Thanks for reworking this. :)

@plnordquist
Copy link
Author

I've updated the versions now. These changes might be worth a minor version bump but I've bumped the versions as per your comment.

I should have said in the first post but there is an issue with Ansible failures where it will immediately attempt another reconciliation so the operator kind of gets stuck attempting to create a tenant when a referenced flavor does not exist, see https://github.com/operator-framework/operator-sdk/issues/2502.

@kfox1111
Copy link

Yeah. It could be a minor version bump, as it does add a little bit of functionality with the namespace labels. Its < 1 though, so probably close enough.

I think the operator misbehaved when flavor was nonexistent before too. So probably not a regression. Since normal users cant do TenantNamespaces, its good enough for now.

@kfox1111 kfox1111 merged commit 5ee82b7 into pnnl-miscscripts:master May 12, 2020
@plnordquist plnordquist deleted the tno-helm-module branch May 14, 2020 18:28
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.

2 participants