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

feat(kubernetes): make reconcile support arrays #112

Merged
merged 4 commits into from
Nov 15, 2019
Merged

Conversation

sh0rez
Copy link
Member

@sh0rez sh0rez commented Nov 14, 2019

Adds array support for Reconcile.

Also documents the expected jsonnet structure. Fixes #110

@sh0rez sh0rez added kind/feature Something new should be added component/kubernetes Working with Kubernetes clusters labels Nov 14, 2019
@sh0rez sh0rez requested a review from rfratto November 14, 2019 22:01
@sh0rez sh0rez self-assigned this Nov 14, 2019

Tanka evaluates the `main.jsonnet` file of your [Environment](/environments) and
filters the output (either Object or Array) for valid Kubernetes objects.
An object is considered valid if it has both, a `kind` and a `metadata.name` set.
Copy link
Contributor

@lawrencejones lawrencejones Nov 15, 2019

Choose a reason for hiding this comment

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

It's usually the case that Kubernetes deals in GVK (Group-Version-Kind) + Name tuples when uniquely identifying resources. It's probably better for tanka to output any leaf node that exhibits the structure:

// Assuming github.com/stretchr/objx
obj.Get("apiVersion").IsStr() &&
  obj.Get("kind").IsStr() &&
  obj.Get("metadata.name").IsStr()

I believe anything other than this format will fail validation anyway.

Copy link
Member

Choose a reason for hiding this comment

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

I agree with @lawrencejones but also this remark seems inaccurate. reconcile.go seems to only be logging for kind and apiVersion right now. We should be checking for all three and document them here.

@lawrencejones
Copy link
Contributor

Hey @sh0rez, I think we have a branch that might do this, along with a few clean-ups we've made along the way. We'll push it in about an hour if you can wait for that!

I think your documentation is looking ace though 👌

@lawrencejones
Copy link
Contributor

#109 is a separate take on this, hopefully with some simplifications that make tanka a bit easier to work on.

Copy link
Member

@rfratto rfratto left a comment

Choose a reason for hiding this comment

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

Mostly LGTM, needs a few changes.


Tanka evaluates the `main.jsonnet` file of your [Environment](/environments) and
filters the output (either Object or Array) for valid Kubernetes objects.
An object is considered valid if it has both, a `kind` and a `metadata.name` set.
Copy link
Member

Choose a reason for hiding this comment

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

I agree with @lawrencejones but also this remark seems inaccurate. reconcile.go seems to only be logging for kind and apiVersion right now. We should be checking for all three and document them here.

docs/jsonnet/expected-structure.md Show resolved Hide resolved
docs/jsonnet/expected-structure.md Show resolved Hide resolved
pkg/kubernetes/reconcile.go Show resolved Hide resolved
Copy link
Member

@rfratto rfratto left a comment

Choose a reason for hiding this comment

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

LGTM

@sh0rez
Copy link
Member Author

sh0rez commented Nov 15, 2019

I agree with @lawrencejones but also this remark seems inaccurate. reconcile.go seems to only be logging for kind and apiVersion right now. We should be checking for all three and document them here.

That's a good idea, just not in this particular PR. Will open an issue, let's address once #111 is done

@sh0rez sh0rez merged commit eb64779 into master Nov 15, 2019
@sh0rez sh0rez deleted the jsonnet-structure branch November 15, 2019 22:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/kubernetes Working with Kubernetes clusters kind/feature Something new should be added
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Document expected structure of Jsonnet output
3 participants