-
Notifications
You must be signed in to change notification settings - Fork 27
Add standalone ansible-cli command and release binaries #150
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
base: main
Are you sure you want to change the base?
Conversation
.goreleaser.yml
Outdated
- "--builder={{ .Env.BUILDX_BUILDER }}" | ||
extra_files: | ||
- "images/ansible-operator/Pipfile" | ||
- "images/ansible-operator/Pipfile.lock" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could we keep the indentation as before to make easier the review?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, I have reverted the indentation changes for easier review.
Makefile
Outdated
.PHONY: ansible-cli | ||
ansible-cli: | ||
@mkdir -p $(BUILD_DIR) | ||
GOOS=$(BUILD_GOOS) GOARCH=$(BUILD_GOARCH) go build $(GO_BUILD_ARGS) -o $(BUILD_DIR)/ansible-cli ./cmd/ansible-cli |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See that we have already a make build and install which is like we have in SDK: https://github.com/operator-framework/operator-sdk/blob/master/Makefile#L83-L95
See here:
https://github.com/operator-framework/ansible-operator-plugins/blob/main/Makefile#L51-L55
AND
https://github.com/operator-framework/ansible-operator-plugins/blob/main/Makefile#L72-L85
So, I think we need to use it.
We just need to generate the bin using it.
Also, we will need to decide the name of the bin as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, I think we need to use it.
Updated the logic to use existing make build
and make install
targets.
Also, we will need to decide the name of the bin as well
I am open to suggestions. Some options could be ansible-scaffold
, ansible-plugin
etc.
Signed-off-by: chiragkyal <ckyal@redhat.com>
d72f5d1
to
f250720
Compare
) | ||
pkg.CheckError("getting cli implementation:", err) | ||
return c | ||
return ansiblecli.GetPluginsCLI() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should not include this code here.
Instead, we must ensure that the hack generator uses the actual binary, rather than mocking the CLI and invoking it directly.
The binary should be built locally and used during generation to validate that everything functions correctly—this is the approach taken in the Operator SDK.
Reference: operator-sdk/hack/generate/samples/generate_testdata.go#L30-L37
c/c @acornett21 ^
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The binary should be built locally and used during generation to validate that everything functions correctly—this is the approach taken in the Operator SDK.
I think the current approach follows a deliberate design choice made in #4 PR, after which we are now generating the testdata on the fly using the helper library.
This PR is trying to unify the testdata generation with the binary's command execution. We created a single entrypoint (cli.GetPluginsCLI()
) that is called by both the main
function for the final binary and by the hack script that generates the test samples.
It ensures that our testdata is always generated using the exact same logic and configuration that the end-user's binary will have. It creates a tight coupling that prevents any possible divergence between the test suite and the released artifact.
This change is a continuation of the existing pattern. I think we are not breaking the testdata generation flow or logic.
@mkdir -p $(BUILD_DIR) | ||
GOOS=$(BUILD_GOOS) GOARCH=$(BUILD_GOARCH) go build $(GO_BUILD_ARGS) -o $(BUILD_DIR) ./cmd/ansible-operator | ||
GOOS=$(BUILD_GOOS) GOARCH=$(BUILD_GOARCH) go build $(GO_BUILD_ARGS) -o $(BUILD_DIR) ./cmd/{ansible-operator,ansible-cli} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought in some names:
ansdk
ansctl
opsible
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/cc: @acornett21 for thoughts.
I liked the ansdk
naming. If we all agree with this, we can rename it.
@@ -4,7 +4,7 @@ | |||
# More info: https://book.kubebuilder.io/reference/project-config.html | |||
domain: example.com | |||
layout: | |||
- go.kubebuilder.io/v1 | |||
- base.ansible.sdk.operatorframework.io/v1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The term "base" currently refers to the default plugin configuration that doesn't use kustomize.v2 which generates the config.dir.
So we might want to change the registration line:
plugin.WithName(ansible.Plugin{}.Name()),
to use a more explicit name for the bundle. Instead of the current form:
base.ansible.sdk.operatorframework.io/v1
we might wnat to align with something like
.ansible.operatorframework.io/v1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the suggestion!
Update the plugin name to reflect ansible.operatorframework.io/v1
Signed-off-by: chiragkyal <ckyal@redhat.com>
Description of the change:
This PR introduces a standalone
ansible-cli
binary to simplify the setup for scaffolding Ansible-based operators.Motivation for the change:
ansible-cli
directly.Implementation Details
New
ansible-cli
Command: A new command (cmd/ansible-cli
) is introduced, which bundles the plugins and necessary logic.Build & Release: The Makefile and
.goreleaser.yml
configurations are updated to build and release theansible-cli
binary.CI Workflow: A new GitHub Actions job,
build-ansible-cli
, is added to ensure the binary builds correctly on each change.E2E: Uses a common CLI entrypoint for
hack/generate/samples/ansible/generate.go
which is used to generate e2e testdata.Example usage
make build/ansible-cli mkdir memcached-operator cd memcached-operator ../ansible-cli init --domain example.com ../ansible-cli create api --group cache --version v1alpha1 --kind Memcached --generate-role