doc/proposal: Ansible Operator proposal #430

shawn-hurley · 2018-08-27T11:53:21Z

Describes how the library packages of ansible operator
will be layed out
New commands to manage ansible operators.

* Describes how the library packages of ansible operator will be layed out * New commands to manage ansible operators.

fanminshi · 2018-08-27T18:30:32Z

doc/proposals/ansible-operator.md

+|  | - cr.yaml
+```
+
+`operator-sdk generate crd <api-version> <kind> ` This will be used to generate new CRDs based on ansible code. The command helps when a user wants to watch more than one CRD for their operator.


We probably want to have this functionality for the go operator as well. I think once we switch to controller runtime, we can do that for the go operator. The question I am having now is that how do we generate additional CRDs for both go type and ansible type?

I think there is already an operator-sdk generate k8s which will generate the code for custom resources. The ansible only needs the CRD. Therefore I suggest that operator-sdk generate k8s both generates the go structures and a CRD, while this command only generates the CRD.

Thoughts?

while this command (operator-sdk generate crd <api-version> <kind>) only generates the CRD.

as long as it can generate a CRD regardless whether the operator is a Go one or Ansible one. I think this command makes sense.

fanminshi · 2018-08-27T18:31:07Z

doc/proposals/ansible-operator.md

+Flags:
+Optional: --defaults-file - A path to the defaults file to use to generate a new CRD.yaml. If this is not defined, then an empty CRD is created.
+
+`operator-sdk up ansible` - This should use the known structure and the ansible operator code to run the operator from this location. The command works by running the operator-sdk binary, which includes the ansible operator code, as the operator process. This is slightly different than the current up local command. For debugging a user could then set a flag --verbose which tells ansible to be more verbose.


This needs a bit thought. Currently, we have operator-sdk up local to run test for go operator. Maybe we can have operator-sdk up local --type=ansible to run ansible operator locally.

Maybe we should discuss the up command. Currently, it only has a local subcommand. We should determine if what other uses of Up there are and determine if another subcommand would be easier to use or an option on local.

I didn't see another subcommand for up so I am having a hard time seeing why another subcommand would be a problem here

I didn't see another subcommand for up so I am having a hard time seeing why another subcommand would be a problem here

Initially, we were thinking about add another command calling up remote such that it can build the operator image, push it to a repo, and then runs it to a remote cluster as discussed here #142 (comment). I am unsure if that's relevant or not anymore. If it is still relevant, then we should group all the commands that uses the usr's computer as a local pod within up local command. For instance, operator-sdk up local --type=Ansible runs the ansible operator locally and operator-sdk up remote --type=Ansible builds the ansible operator and deploys to a remote cluster.

So I have two thoughts

I don't want ansible operator developers to feel like a second-class citizen from the commands. The constant need to add type arguments worries me in this sense because we are not giving them a "default" way of running their code.

The idea of a "remote" command seems like a good idea. Your right that we would want the ansible operator to be used for that command as well.

I wonder if we could do something like:

operator-sdk up {local,remore} - where the presence of a cmd directory with a .go file somewhere inside means we go run, while the absence means we use ansible.

The only issue I worry about with this is a possible poor user experience if their project is in a very odd state. It might be hard for us to determine errors where if they are explicit we can do more error checking.

Thoughts?

magic detection can cause all sorts of problems. Also, assuming one when its not the other also doesn't scale when a 3rd option comes around. I'm for a little more typing that is always consistent. don't make go lang a special thing and just add a required, type flag = [go, ansible]

@fanminshi Are you ok with this? I think I agree with @kfox1111

@shawn-hurley @kfox1111 I think we default the --type flag to go for backward compatibility. However, if user can use --type go if they want to. In essence, we should have a --type for all supported types but with the default to go. That's my opinion on this. If we want to make the flag required, that can cause backward compatibility issue which is also something we can discuss about. I am not for breaking backward compatibility in general. Since the project is still in early stage, I am fine with it if everyone thinks it is a good idea.

mhrivnak · 2018-08-29T20:37:45Z

doc/proposals/ansible-operator.md

+
+`operator-sdk build --type=ansible <image-name>` - This builds the operator image. The command calls ansible-galaxy install for the role dependencies and creates the image from the Dockerfile.
+
+`operator-sdk new --type ansible --hybrid`  - This converts an ansible operator to a new hybrid operator. Hybrid operators allow the developer the ability to create a golang operator while still using the ansible operator code they have already written. This is intended for advanced users to allow developers full customization of their operator. The generated operator should now function the same as standard operator-sdk operators.


I suggest changing this option from --hybrid to --source. The --source option tells the command to include the source code for the ansible operator.

hasbro17 · 2018-09-05T21:12:41Z

doc/proposals/ansible-operator.md

+
+`operator-sdk build --type=ansible <image-name>` - This builds the operator image. The command calls ansible-galaxy install for the role dependencies and creates the image from the Dockerfile.
+
+`operator-sdk new --type ansible --source`  - This converts an ansible operator to a ansible operator. A user can now change the base ansible operator. This will also allow users to continue using their ansible code while also using the operator-sdk. This is intended for advanced users to allow developers full customization of their operator. The generated operator should now function the same as standard operator-sdk operators.


This converts an ansible operator to a ansible operator

Can we reword and clarify this statement a bit?
Does this mean we run operator-sdk new --type ansible --source inside the pure ansible type project previously generated by operator-sdk new --type ansible --kind=<kind> --apiversion<group/version> to convert it to a hybrid type.

Adds clarification about the `--source` command.

fanminshi · 2018-09-06T18:48:11Z

lgtm

hasbro17 · 2018-09-06T21:08:38Z

doc/proposals/ansible-operator.md

+Packages will be added to the operator-sdk. These packages are designed to be usable by the end user if they choose to and should have a well documented public API. The proposed packages are:
+* /operator-sdk/pkg/ansible/controller
+  * Will contain the ansible operator controller.
+  * Will contain a exposed reconciler. But the default `Add` method will use this reconciler.


But the default Add method will use this reconciler

@shawn-hurley Just so I understand this correctly, this controller is expected to be added to the manager in the hybrid case right?
likely in cmd/manager/main.go:

import ac "github.com/operator-framework/operator-sdk/pkg/ansible/controller" ... // Add the manage ac.Add(mgr, ac.Options{...})

So users can either call Add() to add the default ansible controller to their manager.
Or create their own controller and use the default reconciler (AnsibleOperatorReconciler from #461 )

*an exposed

My bad I just realized this controller pkg is being used by up local's manager to run the ansible-operator locally.
Although this can still be used in the hybrid case as well I think.

@hasbro17 that is what we are thinking as well +1

hasbro17 · 2018-09-06T21:38:20Z

doc/proposals/ansible-operator.md

+### Commands
+We are adding and updating existing commands to accommodate the ansible operator.  Changes to the `cmd` package as well as changes to the generator are needed.
+
+`operator-sdk new --type ansible --kind <kind> --api-version <group/version>`  This will be a new generation command under the hood. We will:


Minor nit but this would be:
operator-sdk new <project-name> --type ansible --kind <kind> --api-version <group/version>
if we keep the same behavior of operator-sdk new creating a new project directory.

hasbro17 · 2018-09-06T22:17:01Z

doc/proposals/ansible-operator.md

+Optional: --defaults-file - A path to the defaults file to use to generate a new CRD.yaml. If this is not defined, then an empty CRD is created.
+
+`operator-sdk up local` - This should use the known structure and the ansible operator code to run the operator from this location. This will need to be changed to determine if it is an ansible operator or a golang operator. The command works by running the operator-sdk binary, which includes the ansible operator code, as the operator process. This is slightly different than the current up local command.
+


running the operator-sdk binary, which includes the ansible operator code

Sorry if I'm rehashing this again but how exactly does the operator-sdk run the user's local ansible code?
In a Golang project it would just run the main entry point from the local project exec.Command("go run cmd/manager/main.go").

And for a pure ansible project up local is just going to run the main function inside its own process right? i.e start a new manager, read the local watches.yaml and create a controller for each gvk.

Basically up local would run this internally.
https://github.com/water-hole/ansible-operator/blob/master/cmd/ansible-operator/main.go#L29-L78

Is that right?

In a pure ansible operator, up local would run the code you linked to, which would be already compiled into the operator-sdk binary.

In a "hybrid" operator, up local would just treat it like any other golang operator.

That is correct!

hasbro17 · 2018-09-10T21:17:00Z

LGTM

johnkim76 · 2018-09-12T16:18:35Z

doc/proposals/ansible-operator.md

+\ | - <kind>
+|  \ | - generated ansible role
+|
+| - watchers.yaml


@shawn-hurley shouldn't this be watches.yaml?

yes, oops 🤦‍♂️

cool.. thought it was me :)

proposal: ansible operator proposal

8361a3a

* Describes how the library packages of ansible operator will be layed out * New commands to manage ansible operators.

fanminshi reviewed Aug 27, 2018

View reviewed changes

mhrivnak reviewed Aug 29, 2018

View reviewed changes

updating command to generate a "source" version of ansible operator.

c791abe

hasbro17 reviewed Sep 5, 2018

View reviewed changes

Shawn Hurley added 2 commits September 6, 2018 08:35

Update ansible-operator.md

0417685

Adds clarification about the `--source` command.

Changing proposal based on feedback from offline meeting.

0866292

shawn-hurley mentioned this pull request Sep 6, 2018

[WIP] Initial addition of ansible operator to operator-sdk #461

Closed

hasbro17 reviewed Sep 6, 2018

View reviewed changes

adding project to the new command

4912b86

hasbro17 merged commit a2f2edd into operator-framework:master Sep 11, 2018

johnkim76 reviewed Sep 12, 2018

View reviewed changes


		`operator-sdk build --type=ansible <image-name>` - This builds the operator image. The command calls ansible-galaxy install for the role dependencies and creates the image from the Dockerfile.

		`operator-sdk new --type ansible --hybrid` - This converts an ansible operator to a new hybrid operator. Hybrid operators allow the developer the ability to create a golang operator while still using the ansible operator code they have already written. This is intended for advanced users to allow developers full customization of their operator. The generated operator should now function the same as standard operator-sdk operators.


		`operator-sdk build --type=ansible <image-name>` - This builds the operator image. The command calls ansible-galaxy install for the role dependencies and creates the image from the Dockerfile.

		`operator-sdk new --type ansible --source` - This converts an ansible operator to a ansible operator. A user can now change the base ansible operator. This will also allow users to continue using their ansible code while also using the operator-sdk. This is intended for advanced users to allow developers full customization of their operator. The generated operator should now function the same as standard operator-sdk operators.

		Optional: --defaults-file - A path to the defaults file to use to generate a new CRD.yaml. If this is not defined, then an empty CRD is created.

		`operator-sdk up local` - This should use the known structure and the ansible operator code to run the operator from this location. This will need to be changed to determine if it is an ansible operator or a golang operator. The command works by running the operator-sdk binary, which includes the ansible operator code, as the operator process. This is slightly different than the current up local command.

doc/proposal: Ansible Operator proposal #430

doc/proposal: Ansible Operator proposal #430

Uh oh!

Conversation

shawn-hurley commented Aug 27, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fanminshi commented Sep 6, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hasbro17 commented Sep 10, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!