feat(resources): define operator install strategy spec #11

ericavonb · 2017-08-30T21:37:10Z

Notes

The doc is sparse and only outlines the very basic strategy without much argument. I determined it was more valuable to get a prototype to play with directly rather than over-analyze.

Another reason is that deployment strategies and upgrades are in active discussion for core services and their operators (e.g. tectonic-channel-operator, cluo, etc) .

See design doc: Manifest Generation & Management

However, as issues are discovered and we gather more information as we go, the discussion should continue and the spec updated to reflect current state.

ericavonb · 2017-09-06T18:13:01Z

resources/almoperator.operatorversion.yaml

+    kind: deployment
+    items:
+      apiVersion: extensions/v1beta1
+      kind: Deployment


@charltonaustin I c/p'd this from the helm deployment template and plugged in values. Will this work or do we need to parameterize a bit, use a config map or something?

josephschorr · 2017-09-06T18:19:08Z

docs/operator-install-strategy.md

+            - name: vault-operator
+              image: quay.io/coreos/vault-operator:0.0.2
+              env:
+              - name: MY_POD_NAMESPACE


Would this namespace be pre-defined? If so, this might conflict with the namespace in which the app is running; we may have no choice but to make this partially templated

That's what I'm afraid of 😞 . config maps give an easy way to get around templating the deployment but does add the complexity of managing config files. Here's how prometheus is handling it right now: https://coreos.com/tectonic/docs/latest/tectonic-prometheus-operator/user-guides/configuring-tectonic-monitoring.html#reference

@jzelinskie, @ecordell: Is there a "standard" templating system for Kub objects outside of Helm?

There isn't really one: kubernetes/kubernetes#23896

That's an attempt to make a standard one that resulted in a list of client-side options instead.

I don't think we should get involved in the templating game.
It is not a thing that will be standardized or resolved any time soon.

Is there a "standard" templating system for Kub objects outside of Helm?

There was an attempt (dropped) to upstream one: https://github.com/kubernetes/community/blob/master/contributors/design-proposals/templates.md

I don't think we should get involved in the templating game.

The number of projects listed by @ericavonb reflect a strong requirement by the community to have some way to easily parametrize deployments.
Being involve doesn't necessary include invent a new thing, but at least leverage an existing one.

I've heard from a few people at coreos that they wish we'd be opinionated on this and just pick one. There's a point at which deciding to putting off deciding adds up to more worry than deciding. Hard call on where that line is thought.

@ant31 if we were to pick something and run with it, what would you recommend?

I think it's worth bringing in some of the original borg/k8s author's thoughts, since those will probably direct the conversation upstream:

We believe the most effective approach is to accept this need, embrace the inevitability of programmatic configuration, and maintain a clean separation between computation and data. The language to represent the data should be a simple, data-only format such as JSON or YAML, and programmatic modification of this data should be done in a real programming language, where there are well-understood semantics, as well as good tooling.

(from http://queue.acm.org/detail.cfm?id=2898444)

Is there any way we can avoid "real" templating for now?

Can the valuesFrom and matchExpressions style value-plucking that already exists in kubernetes work for now?

P.S. That same article identifies "dependency management" of infrastructure services as being a "hard problem" that should be solvable on top of kubernetes ;)

Let's punt on configuration for now; I don't believe we don't need a configurable deployment for the alpha

ecordell · 2017-09-06T18:47:58Z

resources/almoperator.operatorversion.yaml

@@ -9,4 +9,35 @@ spec:
  - requirementType: customresourcedefinition
    resourceDefinitionLink: /apis/apiextensions.k8s.io/v1beta1/customresourcedefinitions/apptypes.app.coreos.com
  installStrategy:


I think if this is the way we're going to go it makes more sense to have installStrategy be an array (instead of kind: deployment having an array of deployments).

That will make it easier to validate as well, you can use the anyOf and have different schemas for kind: deployment and kind: image

I thought about that. My thinking of keeping it a single object to start was to reduce the extra complexity for mvp (which one is used? the first? user specified? etc)

I'm thinking about it from a json-schema perspective. If it's an array of heterogeneous objects, you can use "anyOf" to validate them.

Right now in your schema you can't express that the image field is only required if kind: image is set, which has already led to jimmy writing an example with kind: deployment, image: blah in another PR.

There is a way to do it via a "discriminator", it's not that pretty thought: https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.0.md#discriminatorObject

ecordell · 2017-09-06T18:48:47Z

resources/almoperator.operatorversion.yaml

@@ -9,4 +9,35 @@ spec:
  - requirementType: customresourcedefinition
    resourceDefinitionLink: /apis/apiextensions.k8s.io/v1beta1/customresourcedefinitions/apptypes.app.coreos.com
  installStrategy:
-    todo: EvB
+    kind: deployment


I think a different word would be better than kind, which generally refers specifically to kubernetes Kind's. Maybe type or name?

They're all so overused and confusing 😫 . What do you think of method or strategy?

strategy seems good to me?

ecordell · 2017-09-06T18:49:43Z

resources/almoperator.operatorversion.yaml

-    todo: EvB
+    kind: deployment
+    items:
+      apiVersion: extensions/v1beta1


is it important to have the deployment be a part of OperatorVersion or does it make sense to separate it out?

As in a separate object?

Yeah, as a separate object.

This happened before when we split out the EtcdCluster CRD definition away from the AppType resource, instead saying that AppType expects that CRD to exist. We could do the same here, but I think it gets tricker how to reference it:

if we referenced by a sha then we know every copy in every cluster with that AppType is running the same deployment

if we referenced by a name/uid/ownerReference/etc then we could allow the user to bring their own and hook up the relationship manually, which might be a good way to allow customization (and bypass the templating issue)

ecordell · 2017-09-06T18:50:29Z

resources/operatorversion.crd.yaml

+              type: array
+              description: List of deployments to create
+              items:
+                $ref: '#/definitions/io.k8s.api.apps.v1beta1.Deployment'


is this going to work for validation? doesn't the spec for deployments need to be available at that path?

You're probably right. I took this from the k8s openapi spec, will need to find the full url.

I think if you just find that doc and put it at that relative path in this repo, things will work?

ecordell · 2017-09-06T18:51:52Z

resources/operatorversion.crd.yaml

+            image:
+              type: string
+              description: 'container image to run for kind=image'
+            deployOptions:


this should probably be represented in the example as well.

Also this ties into a question about whether OperatorVersions are configurable, or if there's separate configuration that applies to immutable OperatorVersions. Any thoughts?

This was sort of a placeholder for that discussion/experimentation. I also imagined for updates we might need some configuration (like rolling deploy? upsert? replace?)

The stance so far has been that operator-level config will get pushed down to the managed CRDs (like EtcdCluster) and the individual operators (etcdoperator) will read them and do the right thing.

I would remove this for now unless we know we're going to have deployOptions. I think these examples and schemas should represent our current made decisions and not our thoughts about how we might change them.

ecordell · 2017-09-06T18:53:30Z

docs/operator-install-strategy.md

@@ -0,0 +1,74 @@
+# Operator Install Strategy
+
+### [ALM-76](https://coreosdev.atlassian.net/browse/ALM-76)


I don't have a strong opinion on this, but I've been thinking we were writing things in this repo from the perspective that they had been decided (and would be open source).

I've been working under the assumption it would not be open source, since then we can utilize operator-client and other tooling. Should clarify with rob.

jakedt · 2017-09-07T17:48:35Z

docs/operator-install-strategy.md

+  type: com.tectonic.storage
+spec:
+  operators:
+    cdr: vault


jakedt · 2017-09-07T17:50:41Z

Documentation/design/resources/almoperator.operatorversion.yaml

+    kind: deployment
+    items:
+      apiVersion: extensions/v1beta1
+      kind: Deployment


@ecordell how does this match with your proposal for a permission-free or permission-restricted ALM?

I think this is fine, ALM just needs permission to create deployments.

Alternately, it could be surfaced to a user, to create the deployment with their credentials.

How does it work for permissions on things that a deployment can define on its pod spec? E.g. persistent volumes?

I think this is going to be a thing that has to be granted per use-case.

Some users might have enough access to create a deployment with persistentvolumes. Other users might not, and an admin wants to restrict by granting the ability to create persistentvolumes to the ALM itself, on a users behalf.

ecordell · 2017-09-07T19:13:24Z

docs/operator-install-strategy.md

+e.g.
+
+```yaml
+apiVersion: app.coreos.com/v1beta1


This should be an operatorversion right? and the pre-amble should match the latest version?

ecordell · 2017-09-07T19:14:50Z

Documentation/design/resources/operatorversion.crd.yaml

+            image:
+              type: string
+              description: 'container image to run for kind=image'
+            deployOptions:


The stance so far has been that operator-level config will get pushed down to the managed CRDs (like EtcdCluster) and the individual operators (etcdoperator) will read them and do the right thing.

I would remove this for now unless we know we're going to have deployOptions. I think these examples and schemas should represent our current made decisions and not our thoughts about how we might change them.

charltonaustin · 2017-09-08T17:44:12Z

Documentation/design/resources/almoperator.operatorversion.yaml

+          spec:
+            containers:
+              - name: alm
+                image: quay.io/quay/alm:master


This guy has been updated... do we have a way of keeping these in sync?

josephschorr · 2017-09-11T19:13:19Z

Documentation/design/resources/almoperator.operatorversion.yaml

+      kind: Deployment
+      metadata:
+        name: alm-operator
+        namespace: alm-app


Namespace needs to be inherited from the namespace of the OperatorVersion. @ecordell: If we leave it off of here, can we have it automatically inherit?

josephschorr · 2017-09-11T19:14:05Z

docs/operator-install-strategy.md

+            - name: vault-operator
+              image: quay.io/coreos/vault-operator:0.0.2
+              env:
+              - name: MY_POD_NAMESPACE


Let's punt on configuration for now; I don't believe we don't need a configurable deployment for the alpha

ericavonb force-pushed the ericavonb/ALM-76/operator-install-strategy branch 3 times, most recently from 279d5d4 to bf8123a Compare September 6, 2017 17:02

ericavonb changed the title ~~WIP operator install strategy~~ feat(resources): define operator install strategy spec Sep 6, 2017

ericavonb force-pushed the ericavonb/ALM-76/operator-install-strategy branch 3 times, most recently from 1bb86b0 to a575258 Compare September 6, 2017 17:38

ericavonb requested review from ecordell and charltonaustin September 6, 2017 17:39

ericavonb commented Sep 6, 2017

View reviewed changes

josephschorr reviewed Sep 6, 2017

View reviewed changes

ecordell requested changes Sep 6, 2017

View reviewed changes

EvB added 2 commits September 6, 2017 15:45

doc(operator-install-strategy): think out install strategy

272ba89

feat(resources/installstrategy): add strategy schema

b6ec0c0

ericavonb force-pushed the ericavonb/ALM-76/operator-install-strategy branch from a575258 to b6ec0c0 Compare September 6, 2017 19:47

jakedt reviewed Sep 7, 2017

View reviewed changes

doc(operator-install-strategy): s/cdr/crd

11ddd90

ecordell reviewed Sep 7, 2017

View reviewed changes

charltonaustin reviewed Sep 8, 2017

View reviewed changes

josephschorr reviewed Sep 11, 2017

View reviewed changes

ecordell mentioned this pull request Sep 15, 2017

Add install strategies #50

Merged

ecordell closed this Sep 15, 2017

ericavonb deleted the ericavonb/ALM-76/operator-install-strategy branch September 19, 2017 19:41

		@@ -0,0 +1,74 @@
		# Operator Install Strategy

		### [ALM-76](https://coreosdev.atlassian.net/browse/ALM-76)

feat(resources): define operator install strategy spec #11

feat(resources): define operator install strategy spec #11

Conversation

ericavonb commented Aug 30, 2017 • edited Loading

Notes

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ecordell Sep 7, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ericavonb commented Aug 30, 2017 •

edited

Loading

ecordell Sep 7, 2017 •

edited

Loading