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

Implement automation via kyaml setters2 #23

Merged
merged 10 commits into from
Oct 21, 2020
Merged

Implement automation via kyaml setters2 #23

merged 10 commits into from
Oct 21, 2020

Conversation

squaremo
Copy link
Member

@squaremo squaremo commented Oct 6, 2020

This implements the design given in fluxcd/flux2#107 (comment); briefly, you mark fields in your YAMLs with an image policy name, and the automation sets those fields (wherever they are) to the latest image for the given policy.

This works but needs some polish. It's not obvious, for example, why something doesn't get updated when you think it should. I also need to try it with a few example configs; e.g., with a kustomization, and with a custom resource.

One choice that was forced upon me: I want the markers to have both a namespace and name of the corresponding image policy, to avoid collisions, but / is unavailable as a separator, since it's used in OpenAPI ref paths (and that's what the marker is, a ref). I've used :, but any character that's not allowed in resource names and isn't / either would do.

I've adapted the instructions given in the README for trying this out.

TODO (not necessarily in this PR):

  • respect the paths field from the spec
  • better troubleshooting -- it's hard to work out why it's not working when it's not working
  • ability to replace the tag value rather than the whole image ref (per Stefan's question)
  • tool to add marker? or at least, the lib for integration into gotk
  • update to v1beta1 (or do that in another PR and rebase here)

This is the means for telling the controller to use kyaml setters, per
the design in
fluxcd/flux2#107 (comment)
This rearranges the test file a little, so that the different
strategies can share some setup.
(this test fails, since the implementation is still a stub)
Previously: replace the YNode (yaml.Node) with a new one which is just
the replacement string.
Now: change the value of the YNode, and set it back in place.
@stefanprodan
Copy link
Member

stefanprodan commented Oct 6, 2020

@squaremo I can't tell from code, can we use a setter to update tags that are not in the same node as the image? e.g.

apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
resources:
  - deployment.yaml
images:
  - name: fluxcd/kustomize-controller
    newName: fluxcd/kustomize-controller
    newTag: v0.1.0 # {"$imagepolicy": "gotk-system:control-plane"}

@squaremo
Copy link
Member Author

squaremo commented Oct 6, 2020

@squaremo I can't tell from code, can we use a setter to update tags that are not in the same node as the image? e.g.

[...example...]

That's something in the design I've not implemented yet -- separating out the bits of the latest image, so you can choose the bit you want. It would look something like this:

apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
resources:
  - deployment.yaml
images:
  - name: fluxcd/kustomize-controller
    newName: fluxcd/kustomize-controller
    newTag: v0.1.0 # {"$imagepolicy": "gotk-system:control-plane:tag"}

(NB the :tag appended to the marker ref; arbitrarily, I've just used a colon as the separator again)

@stefanprodan
Copy link
Member

(NB the :tag appended to the marker ref; arbitrarily, I've just used a colon as the separator again)

Awesome 💯

In particular, insert the setter reference into the deployment file.
@squaremo squaremo force-pushed the use-kyaml-setters2 branch from e7e11f5 to 500036f Compare October 8, 2020 16:20
This adds another means of updating files to the package pkg/update/,
in setters.go (and gives the existing file a better name).

In passing, I changed the test util for comparing before/after
updates, in pkg/files/, to give a little more context when comparing
file contents; and, since the comparison between actual and expected
is not symmetrical, I corrected the order of the args in the tests.
(and update the test to suit)
It's difficult to test the files comparison if it contains all its own
assertions, since you can only test successes. This commit rewrites
the helper to use a func without assertions, that can be more roundly
tested.
It will be useful, for kustomizations e.g., to be able to set just the
tag or just the name (repository). This commit adds setters for those
to the schema -- they have the name of the image setter plus a suffix
of `:tag` or `:name`. For example:

    newName: ubuntu # {"$imagepolicy": "ns:policy:name"}
    newTag: 18.10   # {"$imagepolicy": "ns:policy:tag"}
Copy link
Member

@stefanprodan stefanprodan left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

@squaremo squaremo merged commit 7318ecb into main Oct 21, 2020
@squaremo
Copy link
Member Author

squaremo commented Oct 21, 2020

Merging, and making issues for those TODOs ... (done)

@squaremo squaremo deleted the use-kyaml-setters2 branch October 21, 2020 10:14
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