stub in a best guess at cluster level configuration #124

deads2k · 2018-11-08T16:54:30Z

Cluster level configuration is a stable, discoverable API in the config.openshift.io group that a cluster admin will expect to use to interact and configure his cluster. Placing it in one location allows multiple operators and binaries to depend on a single source of truth for information. It also enable doc-less discovery by a cluster-admin and the divisions here allow individual teams to own their configuration inside of a cluster. Coupling across multiple processes and future potential for subdivision will become clear.

A cluster-admin will be able to go through a flow like

I want to configure a thing. What options are available?
oc api-resources --api-group=config.openshift.io - produces a list of high level features like Images, Builds, Networking, IdentityProvider, etc.
oc explain networking.config.openshift.io - produces a list of API files and their documentation (pull open to kube)
oc edit networking.config.openshift.io - to make a change

There is another set of actors as well. Many settings are actually observed from cluster and cannot reasonably be provided or set by a cluster-admin. For instance, the internalRegistryHostname is known by the image-registry-operator, not the cluster-admin. To represent this, configuration objects have a spec/status split. Controller/Operator maintained information lives in status, cluster-admin maintained information lives in spec. You should not have a field that multi-writer. If multiple writers, especially one machine and one human, try to coordinate writes on a single field, someone will get confused.

The divisions will be along feature lines, not teams or binaries. An operator can observe changes to these types to drive behavior. That observation and wiring is expected to be performed by the feature own in all the binaries that need to react to changes. For instance, if a change to the network configuration needs to be observed and handled by the kube-apiserver, openshift-apiserver, and openshift-controller-manager, the networking team will make the configuration available and manage the wiring in the individually affected processes.

The expected flow goes something like this:

cluster-admin updates config object foo
operator bar observes the parts it cares about into parameters for the operator configuration resource. this provides a configuration level (generation), that can later be checked against status.
the operator resource change is observed by the same operator bar and operator bar writes a new configuration for the operand (binary being managed)
the operand consumes the configuration and behaves as requested by the admin via the cluster-config
operator bar indicates the operand is using the requested level in its status (status.generation)

The API for these types will be the main entrypoint of choice for a cluster-admin and must remain stable across releases. This is in contrast to the on-disk formats for particular binaries which will no longer need stability guarantees since they are operator managed.

This pull provides a first cut at the different buckets of configuration that known today.

/assign @smarterclayton @jwforres @derekwaynecarr

ironcladlou · 2018-11-08T17:21:38Z

config/v1/types_cloudprovider.go

+	Status CloudProviderStatus `json:"status"`
+}
+
+type CloudProviderSpec struct {


The lowest common denominator I know of here would be:

Name string `json:"name"`

Where Name maps to the kube-controller-manager --cloud-provider argument.

Or Provider.

ironcladlou · 2018-11-08T17:24:08Z

config/v1/types_dns.go

+// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
+
+// DNS holds cluster-wide information about DNS.  The canonical name is `cluster`
+type DNS struct {


Does this represent cluster DNS config or external DNS config?

For cluster DNS, the minimal info would be:

ClusterDomain *string `json:"clusterDomain"`

Where ClusterDomain maps to the kubelet --cluster-domain argument. This is almost certainly immutable for the foreseeable future.

I think that if enough of this overlaps with Networking that we should consider putting it there. Service CIDR and Internal network domain are fundamental network things that everyone must respect.

ironcladlou · 2018-11-08T17:25:23Z

config/v1/types_network.go

+	Status NetworkStatus `json:"status"`
+}
+
+type NetworkSpec struct {


@squeed was just talking about this...

ironcladlou · 2018-11-08T17:26:02Z

config/v1/types_network.go

+}
+
+type NetworkSpec struct {
+	// serviceCIDR


Use case: cluster-dns-operator picks a static cluster IP from this range.

ironcladlou · 2018-11-08T17:27:13Z

config/v1/types_routing.go

+// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
+
+// Routing holds cluster-wide information about Routing.  The canonical name is `cluster`
+type Routing struct {


My guess is this should be Ingress instead.

ironcladlou · 2018-11-08T17:29:57Z

config/v1/types_routing.go

+	Status RoutingStatus `json:"status"`
+}
+
+type RoutingSpec struct {


See ClusterIngressSpec

squeed · 2018-11-08T17:34:28Z

Hm. So should we work towards deprecating the cluster-network-operator CRD and configure it exclusively via this API? Otherwise we have a nasty duplicated data problem.

deads2k · 2018-11-08T18:29:59Z

Hm. So should we work towards deprecating the cluster-network-operator CRD and configure it exclusively via this API? Otherwise we have a nasty duplicated data problem.

You can observe the value from here and bump spec in your operator resource to be able to have a single configuration level to determine whether the operator has observed it. See https://github.com/openshift/cluster-kube-apiserver-operator/blob/master/pkg/operator/observe_config.go#L197-L258 for example.

smarterclayton · 2018-11-08T20:14:13Z

config/v1/types_cloudprovider.go

+// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
+
+// CloudProvider holds cluster-wide information about CloudProvider.  The canonical name is `cluster`
+type CloudProvider struct {


We might want to call this InfrastructureProvider.

Or just Infrastructure

deads2k · 2018-11-08T20:25:37Z

Names updated. @smarterclayton @ironcladlou I'm looking to get the categories merged and then looking to have individual feature owners start filling them in.

smarterclayton · 2018-11-08T20:27:26Z

config/v1/types_dns.go

+// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
+
+// DNS holds cluster-wide information about DNS.  The canonical name is `cluster`
+type DNS struct {


I think that if enough of this overlaps with Networking that we should consider putting it there. Service CIDR and Internal network domain are fundamental network things that everyone must respect.

smarterclayton · 2018-11-08T20:28:01Z

config/v1/types_ingress.go

+// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
+
+// Ingress holds cluster-wide information about Ingress.  The canonical name is `cluster`
+type Ingress struct {


We should definitely reserve this name.

smarterclayton · 2018-11-08T20:28:59Z

config/v1/types_oauth.go

+// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
+
+// OAuth holds cluster-wide information about OAuth.  The canonical name is `cluster`
+type OAuth struct {


Possible this should be part of an "Authentication" object instead of separate. I don't want too deep/complex objects, but 3-7 feels like a nice total number to avoid user fatigue.

Possible this should be part of an "Authentication" object instead of separate. I don't want too deep/complex objects, but 3-7 feels like a nice total number to avoid user fatigue.

Authentication configuration is distinct from the configuration of the oauth server. OAuth and IDP may be worth collapsing, but not with Authentication too.

smarterclayton · 2018-11-08T20:29:51Z

config/v1/types_project.go

+// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
+
+// Project holds cluster-wide information about Project.  The canonical name is `cluster`
+type Project struct {


Would probably say this becomes SelfService but we don't have to create it yet. Need to think about it.

smarterclayton · 2018-11-08T20:30:23Z

config/v1/types_scheduling.go

+}
+
+type SchedulingSpec struct {
+	// default node selector (I would be happy to see this die....)


Node selector is the most useful security thing we have done so far, so while I know you hate it... :)

Node selector is the most useful security thing we have done so far, so while I know you hate it... :)

I hate that we have two incompatible implementations that have existed for years and never been cleaned up.

Yeah, but both of those are global security config. Either way we agree.

smarterclayton · 2018-11-08T20:31:09Z

Probably should clearly mark "maybe objects" with a comment indicating that they are subject to change (anything outside of the ones we've decided on like Build and Image).

deads2k · 2018-11-08T20:48:17Z

Probably should clearly mark "maybe objects" with a comment indicating that they are subject to change (anything outside of the ones we've decided on like Build and Image).

done

smarterclayton · 2018-11-08T21:21:00Z

/lgtm

We can iterate in practice

openshift-ci-robot · 2018-11-08T21:21:08Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: deads2k, smarterclayton

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [deads2k,smarterclayton]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

derekwaynecarr · 2018-11-09T02:18:01Z

What happened to Cloud? It seemed useful as long as we handled future case where “external” could be respected to only have meaning for kubelets. We just deferring to future PR?

smarterclayton · 2018-11-09T05:09:53Z

Cloud == Infrastructure.

…

On Thu, Nov 8, 2018 at 9:18 PM Derek Carr ***@***.***> wrote: What happened to Cloud? It seemed useful as long as we handled future case where “external” could be respected to only have meaning for kubelets. We just deferring to future PR? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#124 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABG_p42huNwlu5M_wcTbPDPRIdXuIcoxks5utOXagaJpZM4YVE8J> .

openshift-ci-robot assigned derekwaynecarr, jwforres and smarterclayton Nov 8, 2018

openshift-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Nov 8, 2018

openshift-ci-robot requested a review from smarterclayton November 8, 2018 16:54

ironcladlou reviewed Nov 8, 2018

View reviewed changes

smarterclayton reviewed Nov 8, 2018

View reviewed changes

generated

8c036bb

deads2k force-pushed the config-10-best-guess branch from 765113f to dbf2acc Compare November 8, 2018 20:24

smarterclayton reviewed Nov 8, 2018

View reviewed changes

stub in a best guess at cluster level configuration

a82fcd5

deads2k force-pushed the config-10-best-guess branch from dbf2acc to a82fcd5 Compare November 8, 2018 20:48

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Nov 8, 2018

openshift-merge-robot merged commit c3b47bf into openshift:master Nov 8, 2018

Miciah mentioned this pull request Nov 26, 2018

Add ingress domain #139

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

stub in a best guess at cluster level configuration #124

stub in a best guess at cluster level configuration #124

deads2k commented Nov 8, 2018 •

edited

Loading

ironcladlou Nov 8, 2018

smarterclayton Nov 8, 2018

ironcladlou Nov 8, 2018

smarterclayton Nov 8, 2018

ironcladlou Nov 8, 2018

ironcladlou Nov 8, 2018

ironcladlou Nov 8, 2018

ironcladlou Nov 8, 2018 •

edited

Loading

squeed commented Nov 8, 2018 •

edited

Loading

deads2k commented Nov 8, 2018

smarterclayton Nov 8, 2018

smarterclayton Nov 8, 2018

deads2k commented Nov 8, 2018

smarterclayton Nov 8, 2018

smarterclayton Nov 8, 2018

smarterclayton Nov 8, 2018

deads2k Nov 8, 2018

smarterclayton Nov 8, 2018

smarterclayton Nov 8, 2018

deads2k Nov 8, 2018

smarterclayton Nov 8, 2018

smarterclayton commented Nov 8, 2018

deads2k commented Nov 8, 2018

smarterclayton commented Nov 8, 2018

openshift-ci-robot commented Nov 8, 2018

derekwaynecarr commented Nov 9, 2018

smarterclayton commented Nov 9, 2018 via email

stub in a best guess at cluster level configuration #124

stub in a best guess at cluster level configuration #124

Conversation

deads2k commented Nov 8, 2018 • 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

ironcladlou Nov 8, 2018 • edited Loading

Choose a reason for hiding this comment

squeed commented Nov 8, 2018 • edited Loading

deads2k commented Nov 8, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

deads2k commented Nov 8, 2018

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

smarterclayton commented Nov 8, 2018

deads2k commented Nov 8, 2018

smarterclayton commented Nov 8, 2018

openshift-ci-robot commented Nov 8, 2018

derekwaynecarr commented Nov 9, 2018

smarterclayton commented Nov 9, 2018 via email

deads2k commented Nov 8, 2018 •

edited

Loading

ironcladlou Nov 8, 2018 •

edited

Loading

squeed commented Nov 8, 2018 •

edited

Loading