- 
                Notifications
    You must be signed in to change notification settings 
- Fork 29
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.
| build: ## Build ansible-operator and ansible-cli. | ||
| @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.
| 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>
| PR needs rebase. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. | 
Description of the change:
This PR introduces a standalone
ansible-clibinary to simplify the setup for scaffolding Ansible-based operators.Motivation for the change:
ansible-clidirectly.Implementation Details
New
ansible-cliCommand: A new command (cmd/ansible-cli) is introduced, which bundles the plugins and necessary logic.Build & Release: The Makefile and
.goreleaser.ymlconfigurations are updated to build and release theansible-clibinary.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.gowhich 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