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 commands for image automation API #538

Merged
merged 13 commits into from
Dec 11, 2020
Merged

Conversation

squaremo
Copy link
Member

@squaremo squaremo commented Dec 1, 2020

This will add commands of all the expected varieties, for dealing with the image automation API.

EDIT: removed the dithering over auto vs image; the latter is more fluent (and less typing).

NB the ImagePolicy object does not get {reconcile,suspend,resume} subcommands, since it doesn't follow the reconcile/suspend protocol. It's sufficient at present for the ImageRepository to have these.

@squaremo squaremo force-pushed the image-controller-commands branch 4 times, most recently from 40ecb80 to ac59f8e Compare December 7, 2020 13:19
cmd/flux/get.go Outdated Show resolved Hide resolved
cmd/flux/get.go Outdated Show resolved Hide resolved
@squaremo
Copy link
Member Author

squaremo commented Dec 11, 2020

Pinned the two image-* API module versions to v0.1.0, and rebassed on main.
LATER: removed the --path flag from the create image update command.

This adds all the standard subcommands for the ImageRepository type.

Following `source`, I have put them under a namespace: `auto`,
referring to automation.

NB For `create` I use controllerutil.CreateOrUpdate, which looks to me
like a slightly more rounded version of the upsert* funcs.

Signed-off-by: Michael Bridgen <michael@weave.works>
Signed-off-by: Michael Bridgen <michael@weave.works>
This factors the get command implementation so that the control flow
is generic and relies on a handful of methods, then uses that to add
`get auto image-policy` and to rewrite `get auto image-repository`.

Signed-off-by: Michael Bridgen <michael@weave.works>
This adds a command for deleting ImagePolicy objects. Since the
control flow for the command needs only a runtime.Object (and a name
for the type), it can be factored out.

I have made the argument (field in the deleteCommand struct) an
interface `objectContainer`, through which the command code gets a
`runtime.Object` to deserialise into (and delete). It could be simply
a `runtime.Object` here; however things like `getCommand` require
other methods, so it's convenient to have an interface for it.

Signed-off-by: Michael Bridgen <michael@weave.works>
The export command works the same way for most (all?) types. I have
made it generic and moved it into export.go, then ported
{export,create}_auto_image{repository,policy}.go to use it.

Signed-off-by: Michael Bridgen <michael@weave.works>
This adds the create subcommand, without attempting any refactoring.

NB the TODO: the image/v1alpha1 API does not yet export a const for
the name of the kind. The field `RunInterval` will likely be changed
to `Interval` (with a value field), at some point, too.

Signed-off-by: Michael Bridgen <michael@weave.works>
This uses the established abstractions to implement the usual
subcommands for the ImageUpdateAutomation type.

I've called the sub-subcommand in each case `image-update`, as a
fairly safe shorthand for the much longer `image-update-automation`.

Signed-off-by: Michael Bridgen <michael@weave.works>
Since the generic commands tend to share a few of the methods they
need -- at least AsClientObject -- it's worth having just one wrapper
struct for each API type, and adding methods to it where necessary.

For the automation types, I put these in auto.go.

While doing this I also did some tidying:

 - I changed the name of the wrappers to `<type>Adapter`, and the
   generic adapter to `universalAdapter` (it's only needed for delete,
   so far).

 - I de-exported and renamed some interface methods e.g.,
   `exportItem`. They aren't needed outside the package.

Signed-off-by: Michael Bridgen <michael@weave.works>
.. and refactor. These are all amenable to the adapter refactoring
that has served well so far.

Signed-off-by: Michael Bridgen <michael@weave.works>
Most commands use either a kind, or a more readable spelling of a
kind, in their output. To make this easier, this centralises the
definition of those names in one place, and lets the command
implementations choose whichever they need.

Signed-off-by: Michael Bridgen <michael@weave.works>
This means all the sub-subcommands can drop the `image-` prefix,
making them shorter and more fluent.

E.g.,

    flux create image policy

rather than

    flux create auto image-policy

Signed-off-by: Michael Bridgen <michael@weave.works>
I implemented the isReady procedure for adapters for resume -- use it
in create too.

Signed-off-by: Michael Bridgen <michael@weave.works>
It's a common pattern in the create commands to construct a value,
then (if not exporting it) upsert it and wait for it to
reconcile. This commit factors `upsert`, which does the update/insert
bit, and `upsertAndWait`, which does the whole thing.

Since these output messages, they are methods of `apiType` (previously
`names`), so that they have access to the name of the kind they are
operating on.

Signed-off-by: Michael Bridgen <michael@weave.works>
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

Happy holidays @squaremo 🎄

@squaremo squaremo merged commit 0ba6fc1 into main Dec 11, 2020
@squaremo squaremo deleted the image-controller-commands branch December 11, 2020 16:45
@squaremo
Copy link
Member Author

The gift of a PR review! 🎁 🎄 🎁

squaremo added a commit that referenced this pull request Feb 4, 2021
This slipped through the auto->image change made in the course of
preparing #538.

Signed-off-by: Michael Bridgen <michael@weave.works>
ybelleguic pushed a commit to ybelleguic/flux2 that referenced this pull request Jan 9, 2023
Update file close operation to not use defer and add test case for CopyFromPath
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