Repository navigation
Use operator-sdk binary as operator binary for helm and ansible #860
Description
Feature Request
Is your feature request related to a problem? Please describe.
The ansible operator and helm operator each need to build their own binary that will be run in production. Currently each type has to maintain (almost certainly in a separate repo) its own main.go, Gopkg.toml, Gopkg.lock, etc and facilitate a build and test workflow. The code in main.go overlaps with the operator-sdk up local feature.
Even if we use scaffolds inside the operator-sdk repo to generate all of the above files, their rendered artifacts still need to be maintained somewhere for reproducible builds.
Perhaps most importantly, the above workflow necessarily requires that each binary has its own lifecycle. Each would have its own versioned releases (even if we version them in lock-step), including features, bugs, bug fixes, vendored dependencies (hopefully a subset of what's in operator-sdk), etc.
Describe the solution you'd like
Since all the functionality of helm and ansible operators is already built into the operator-sdk binary, I propose that we add a command that simply runs the correct operator. It will be very similar to up local, but without the "local" parts. Some options that come to mind are:
operator-sdk up
operator-sdk run
With this capability, the operator-sdk binary itself can remove the need to build separate binaries. There will be one binary with one lifecycle.
Activity
AlexNPavel commentedon Dec 18, 2018
That could work. The main disadvantage I see is that the binary would be larger. The ansible operator binary is currently ~38M, helm is ~52M, and the SDK is ~56M. This change might also increase the size of the sdk binary compared to what it is now.
Other than the size though, it would be nice having a single binary handle the sdk, go, ansible, and helm and not require separate code for the base images.
joelanford commentedon Dec 19, 2018
I like this idea.
IMO the binary size differences don't outweigh the benefit of having one binary, one lifecycle, and one implementation of each of the operators' entrypoints. And if I understand correctly, the SDK binary shouldn't grow much (if at all) since it already contains the
up localimplementations.@mhrivnak Would you envision the CLI to be explicit (e.g.
operator-sdk run ansible) or will we auto-detect the operator type in this scenario as well? I personally think it should be explicit.shawn-hurley commentedon Dec 19, 2018
👍 on the explicit for me as well
mhrivnak commentedon Dec 19, 2018
Since all the type-specific code is already compiled in the sdk binary for use by "up local", I think there would be very little change in size after adding this command.
AlexNPavel commentedon Dec 19, 2018
Yeah, looking at the
up localcode, it looks like it's essentially just themain.gofiles for ansible and helm copied over, so that shouldn't change the main binary size. And using the a larger binary for the ansible operator wouldn't really matter much since the base image is so large and the helm binary is almost the same size as the operator-sdk's binary. So the increase in size of the images would be pretty much negligible.This seems like a good idea to me. It would make maintaining the code and images simpler.
hasbro17 commentedon Jan 10, 2019
@mhrivnak @joelanford do we expect users to use the
operator-sdk runcommand as opposed tooperator-sdk up local?From #887 and the discussion above it seems like that's more for the base images to use in their entrypoints.
operator-sdk/pkg/scaffold/ansible/entrypoint.go
Line 48 in 96b06a2
If that's the case then we should clarify in the CLI guide and CHANGELOG that users should still use
operator-sdk up local.joelanford commentedon Jan 10, 2019
@hasbro17 I still expect
operator-sdk up localto continue to be used as it is today for ansible and helm operators. IMO, the distinction is important sinceup localcan make assumptions about the project tree thatruncommands cannot. The differences betweenup localandrunare minimal right now, but the distinction gives us flexibility in the future.👍
mhrivnak commentedon Jan 10, 2019
My understanding:
runis for running the process inside a container that's inside a cluster. This is the operator's "natural environment".runcannot detect the operator type, because it does not have access to its source tree.up localis for a developer to use in order to run the process outside a cluster, where the CWD is the source tree.up localdoes extra things to make this non-native environment look more like its natural environment, such as set environment variables.mhrivnak commentedon Jan 10, 2019
@hasbro17 if there's anything about the naming or the UX you'd like to change, I'm very happy to chat about that, and we can always revise it. Mostly we needed this capability to land quickly so we can nail down the build process for ansible operators.
hasbro17 commentedon Jan 10, 2019
Thank you both for clarifying. That's what i understood from
operator-sdk runas well.@mhrivnak The UX of
runseems good to me. I just wanted to ensure that we document the distinction you've made above betweenrunandup localin a follow up to #887hasbro17 commentedon Jan 18, 2019
Resolved via #887 and #897