Skip to content
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

Initial commit of Kubernetes Application proposal KEP #1629

Closed
wants to merge 4 commits into from
Closed

Initial commit of Kubernetes Application proposal KEP #1629

wants to merge 4 commits into from

Conversation

kow3ns
Copy link
Member

@kow3ns kow3ns commented Jan 19, 2018

This KEP proposed the addition of an Application object to the Kubernetes apps Group.

@k8s-ci-robot k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Jan 19, 2018
@k8s-github-robot k8s-github-robot added sig/apps Categorizes an issue or PR as relevant to SIG Apps. sig/architecture Categorizes an issue or PR as relevant to SIG Architecture. labels Jan 19, 2018
@jbeda
Copy link
Contributor

jbeda commented Jan 20, 2018

Looks reasonable from a KEP process point of view. Some comments on the changes that just landed.

Also -- can you update https://github.com/kubernetes/community/blob/master/keps/NEXT_KEP_NUMBER to reflect that you are taking 3 as the number for this KEP?

Thanks!

/approve no-issue

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 20, 2018
- "@kow3ns"
owning-sig: sig-apps
participating-sigs:
- sig-apps
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We probably don't have to duplicate the owning-sig into the participating-sigs metadata.

editor:
creation-date: 2018-01-15
last-updated: 2018-01-15
status: draft
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is now called accepted to mean that the owning SIG has decided to spend time looking at the topic.


// Components is a map of the applications components to the kinds created by the application (e.g. Pods, Services,
// Deployments, CRDS, etc)
Components map [string]metav1.GroupVersionKind `json:"components,omitempty" protobuf:"bytes,2,opt,name=components"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Explain this better

Copy link

@konryd konryd Jan 23, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to learn more about this too. Looking at the code alone, it would be a way to alias a (GV)Kind within an application.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think clayton means that what is stated in the comment is unclear and should be expounded upon. Which I agree with.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we are going to name every single one of the components, then why not use references? Label selector won't be needed.
This also doesn't allow for resources in other namespaces. What about cluster-wide resources?

Components map [string]metav1.GroupVersionKind `json:"components,omitempty" protobuf:"bytes,2,opt,name=components"`

// Dependencies is a list of other Applications that this Application depends on.
Dependencies []sting `json:"dependencies" protobuf:"bytes,3,opt,name=dependencies"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So doesn't work across namespaces?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lots of applications in the real world will only depend on things in other namespaces

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I too am concerned about seemingly arbitrary restrictions based on namespace. But this is especially apparent with dependencies, where many orgs isolate things like databases in their own namespace.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Concur with the use case pointed out here, but do we need to support it via Application objectl? When one team in an organization, say DBAs, create databases that will exposed for the consumption of another team, why not use Services as the abstraction? Shouldn't the implementation details of an externally managed component be opaque to the consumer? I think that ServiceInstances and ServiceBindings created by a ServiceBroker provided by the DBA are a better model to implement this use case.
Also, there is no guarantee that a user on the other team, say the APIs team, would have correct permissions to view Applications that reside in a namespace controlled by another team. However, we could loosen the restriction for dependencies and allow every organization to work this out themselves via RBAC.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kow3ns those are good points, but then what is the usecase for dependencies at all? Couldn't the use of Services or Service Catalog also be used for dependencies within the same namespace?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The use case is for Dependencies between apps owned by the same organization. Apps with dependencies are white boxes for the users. ServiceInstances and ServiceBindings are black boxes for the consumers. An interesting case is where you are providing a ServiceInstance using an Application. This is sort of a grey box. However, imo, the provider of the ServiceInstance would via the application as a white box, and the consumer would still see it as a black box. These may or may not be the same people.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the design doc should spell out that not all dependencies are expected to be expressed via Dependencies, and that in particular cross-namespace and service-based dependencies need not be.

Selector *metav1.LabelSelector `json:"selector" protobuf:"bytes,4,opt,name=selector"`

// HealthCheck is an optional application level health check.
HealthCheck* ApplicationHealthCheck `json:"healthCheck" protobuf:"bytes,5,opt,name=healthCheck"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why here instead of on service? Adding it to service or another concept is strictly more orthogonal - this is aggregating a bunch of things that seem like they would work better on individual pieces.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The purpose of the Application health check Pod is to establish the readiness of the app as a whole. For instance, this could include establishing that the databases are sufficiently replicated and reachable and that the serving workloads responds to requests at a particular URL. Its included in the application object because its a program that understands the semantics of heath for a specific application.

Moreover, the definition of health may evolve with the version of the Application. My initial though was actually to simply use a URL, the problem here is that, for many use cases, the URL would have to exposed outside of the cluster. I have concerns, from a security perspective, about that.

LicenseURL string `json:"license,omitempty" protobuf:"bytes,11,opt,name=license"`

// DashboardURL is a an optional URL pointing to the applications dashboard.
DashboardURL string `json:"dashboard,omitempty" protobuf:"bytes,12,opt,name=dashboard"`
Copy link
Contributor

@smarterclayton smarterclayton Jan 22, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lots of applications have multiple dashboards. Do I create multiple applications that overlap in order to expose their URLs?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why surface Dashboard and not other URLs that an app may have?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could also imagine links to monitoring and logging dashboards.

It does seem like there may need to be multiple links.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds reasonable. My initial thought was that this would be a landing page that aggregates multiple dashboards for the app, but expanding it to be a container of URLs (potentially mapped to the dashboard name) meets both use cases.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just FYI, this is not always something that can be pre-determined. If I helm install stable/wordpress, I have to wait for the underlying cloud provider to give my WordPress service an external IP/hostname. If I run helm install stable/wordpress --set ingress.hosts.name="mywordpress.com" then it can be pre-determined but only if the installer knows to look for this in the configuration.

I think UIs are better off getting an application URL from the corresponding Service or Ingress resources. This could be a relative URL (e.g. /wp-admin) which a UI could then append to a Service/Ingress URL/host, but it's a difficult association to make if there are multiple Services/Ingresses.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, there may be no exposed Service or Ingress for an Application. What if the Application provides cluster internal functionality and exposes no load balanced Service or Ingress (e.g. RDBMs, MQ, Stream processor).

### Risks and Mitigations

1. In order for the Application API to be useful, we must have broad adoption. In order to mitigate the risk of
a lack of adoption, we will gather consensus from developers in SIG Apps prior to promoting the API to Beta.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're missing one really important one - supporting this is a lot of work for tools, therefore existing tools will ignore this because it doesn't provide enough value. So the proposal needs to do a much better job of justifying why a tool author would adapt.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the basic feedback I've gotten from the UI and tool developers that I've talked to is somewhat contradictory to this. Tectonic is already using CRDs to achieve an App store like experience (beta). Several SIG Apps members have indicated they are internally doing something similar in their own companies using CRDs. My point is there is interest. Again SIG Apps members come from a diverse set of projects in our ecosystem. If the design doesn't meet the threshold for usefulness than we should to modify the design. I will point out that adding the object to a manifest does not require much work and can easily be accomplished problematically.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also have seen a lot of interest in having something that defines an application. What is the criteria needed for this proposal to be accepted?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See @jbeda's comment below


## Graduation Criteria

1. The graduation criteria from Alpha to Beta is simply the consensus of opinion among SIG Apps and SIG CLI that the
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hrm, I don't think this is a good idea. I think we should only go to beta when there is consensus among significant tool authors that this is something they would use.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. I can reword this. SIG Apps members are contributors to/maintainers of Helm, Bitnami, OpenShift, GKE, Azure, Kompose, KSonnet, and Tectonic. My meaning here is precisely that a broad agreement among a diverse set of tool and UI developers to adopt this should be the graduation criteria to Beta. Having it at alpha would give them time to try it out.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we also add consensus amongst the App Definition WG?


- Kubernetes 1.10 - Initial implementation of the Application Kind and Application Controller in the apps/v1alpha
Group Version.
- Kubernetes 1.11 - Promotion of the Application Kind and Application Controller to the apps/v1beta1 Group Version. The
Copy link
Contributor

@smarterclayton smarterclayton Jan 22, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is far too soon. I think if you want to do an aggressive schedule like this, it should be out of core as a CRD, or gather real feedback over a much longer period. 2 months is not enough time.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a notional plan, it may be ambitious, and it certainly contains risk. If we can't hit beta in 1.11 we can update accordingly. As discussed above, I agree that we should promote it to beta only after developers have gotten a chance to test it in alpha clusters, and after they feel that they are ready to produce software to interact with it so that they can gather feedback from their own users.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's worth noting that there is a delay in real world usage after a release. Many folks aren't even using 1.9, yet. Tools take time to make the switch when a new version of k8s comes out, too.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. The reality is that most production clusters won't upgrade to 1.9.x until 1.10.0 is shipped.

### Use Custom Resource Definitions
Rather than introducing an API object, each tool could simply use a CRD (Custom Resource Definition) to represent
Applications. As CRDs are, by definition, a custom API extension mechanism, it is unlikely that this method would
provide for the desired level of interoperability across tools and UIs. If each tool uses their own CRD, they will a
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't follow this line of reasoning. Regular APIs aren't special. If a CRD can't accomplish interop, we will iterate on CRD. There are downsides to CRD, but huge upsides, like you can iterate on them rapidly and demonstrate value before dropping a massive new thing in Kubernetes core.

Note, in general, adding this to kubernetes seems to go against the "limited core" principle we've been advocating for the last year. I think there always be exceptions, but I have yet to see a justification for why you can't build consensus around a CRD before you launch code into kubernetes/kubernetes.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm of the opinion that this should start as a CRD, for similar reasons.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm with @smarterclayton on this: Why not just implement this spec as a CRD, and then push for widespread adoption? There's nothing about CRDs as a technology that causes us to have multiple implementations. It's merely that no single abstraction has yet surfaced as a reasonable solution to the problem.

The argument that "If all tools use a common CRD..." also doesn't follow. It is 100% reasonable to have an oft-used or even ubiquitous Kind that is not in core. In fact, if Kubernetes is to succeed as a platform, we have to get to that point. Otherwise we'll succumb to the same bloat-induced fate of other projects.

On a related note, given that I believe the controller will be more complicated than the proposal seems to indicate, I would hate for "interoperability with Application" to be a new gate for other candidate core features. If it is kept as a separate project, then it can be grown independently of the main Kubernetes source, which is virtuous in this case.

I personally would rather see this implemented as a stand-alone entity that could perhaps be incubated into core if at some point it makes sense. But at this point, I feel like putting it into the core would be a disingenuous attempt to "kill off the competition" before really showing this idea actually meets the needs of Kubernetes operators. Please don't misread this as me saying I don't like the proposal or the idea -- just that I don't see merit in pushing this straight to core.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I should change the wording. It appears I did not clearly communicate my point.
As several organizations are concurrently working on (or have deployed in production) several CRDs that attempt to solve the same problem, we're proposing that it is appropriate to aggregate the lessons learned and design a solution in tree that meets the needs of the community in general.

I would hate for "interoperability with Application" to be a new gate for other candidate core features.

This must, by design, never be the case. As stated in the goals section it must be a strictly opt-in solution.

I personally would rather see this implemented as a stand-alone entity that could perhaps be incubated into core if at some point it makes sense. But at this point, I feel like putting it into the core would be a disingenuous attempt to "kill off the competition"

That should be listed as a goal ... actually perhaps we should have a section called "anti-goals". The reason we are proposing an alpha resource in apps is to leverage the community and infrastructure (e.g. testing, code generation, server side validation, resource versioning) that exists in Kubernetes core.

Based on initial feedback, imo there are enough contributors and interest that we could produce something reasonable, at an alpha level of stability, in the mian repo. If the community as whole feels that this is not the case, then we could consider creating a sub-project and continuing to collaborate across organizations to develop a CRD and third party controller there. Aside from the issues pointed out below, we would largely be operating outside the existing governance of the Kubernetes project. If we think that we should never add this proposal to core, then perhaps that is the right direction.

However, if we think we eventually want a GA Application object in the apps group, if we want to design and implement it as a community, and if we want to ensure that Kubernetes governance is applied to establishing the maturity of the resource as it is promoted through various levels of stability, the process that has precedence is starting with an alpha resource.

To be clear, I'm not saying I disagree with starting as a CRD, but we should carefully consider what we trade if we take this approach. In the same way that OpenShift still supports DeploymentConfig and Deployment, should we eventually add an apps Application resource, we would likely have to support both the CRD and the multiple versions of the core resource for quite some time (here I'm thinking in terms of those organizations that support many Kubernetes clusters at different versions simultaneously).

I don't follow this line of reasoning. Regular APIs aren't special. If a CRD can't accomplish interop, we will iterate on CRD. There are downsides to CRD, but huge upsides, like you can iterate on them rapidly and demonstrate value before dropping a massive new thing in Kubernetes core.

Agree. To me the benefit of in-tree development (as above) is established governance, existing tooling, and an existing repo where we can iterate (potentially more slowly). Shipping the right thing is more important than shipping something (imo).


### Use the Garbage Collector Instead of an Application Controller
We could potentially save core hours and memory by incorporating the control logic of the Application Controller
directly into the Garbage Collector. This design would have a poor separation of concerns and lead to poor code
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ultimately the garbage collector is a specialized graph controller. There needs to be effective code reuse between multiple components doing generalized graph operations on large subsets of the tree, and potentially caching in the future. I wouldn't start with the assumption it should be joined with GC, but I would start with the assumption that out of this should be common code between them.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm saying the opposite here. That it would be bad design to inject more functionality into the GC controller.
I did call out under risks and mitigations that we should mirror the GC controllers implementation. The implication, for a software engineering best practices perspective, is that we would attempt to modularize and share code between the two.

}
// ApplicationSpec is the specification object for an Application. It specifies the Application's components,
// dependencies, and an optional health check. Its selector can be used to select application components.
type ApplicationSpec struct {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general a question I would like to see answered is "why would I want to go to the effort of creating this in every tool, vs having something infer this for me in most cases". The problem with this spec is that I have to do a lot of work to make this useful, which means adhoc users will bypass it unless they get value above and beyond existing concepts (I.e. I can already do deletion with owner refs, I can already list things with labels). I'm not seeing a strong case in the proposal yet to establish that.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, maybe it could be a two sided thing - have a strong relationship model, a CRD to aggregate and add metadata to the tree.

feature is ready to be consumed by tool and UI developers. We expect developers to prototype against the API at alpha
level stability, but, as it will be disabled by default, we do not expect to be able to gather feedback from end users.
2. The graduation criteria from Beta to GA is adoption and interoperability. Before graduating the feature to GA we
would like to see broad adoption by tools (e.g. Helm, Bitnami installer) and UIs (Kubernetes OSS UI, GKE, OpenShift,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some of the OpenShift questions (wearing my hat there for a second) come down to cost vs roi. Tools can auto generate these things. It's just a question of why they would want to. No matter what happens in Helm3, I assume it's going to have an API that is roughly "component" that exposes most of this metadata. Why would I want both a helm deployed object and an application? What's the benefit?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Helm 3 development hasn't started yet, and if we are able to agree on moving forward with this proposal I would strongly suggest that Helm tries to adopt this as a representation for its metadata.


When UIs, either command line or graphical, seek to interact with the application, they can only interact with its
components, and there is no standard way to indicate that those components comprise an application. As previously
mentioned, Many tools in the ecosystem attempt to aggregate the components of an application using labels and/or
Copy link
Contributor

@smarterclayton smarterclayton Jan 22, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discoverability is a good problem to solve. My problem is that this focuses on discoverability by forcing people to create "yet another thing" to track info that is already implicit in the object (i.e. create a new object that also references other objects). Why are we not encouraging an approach that summarizes the existing standard data (references between pods and controllers, pods and services, services and ingress, pods and secrets, secrets and service accounts) and presents that to users?

We have generally tried to build orthogonally useful tools that can be composed well. Application objects seem like the opposite of that.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Taking the example of Helm, we currently have this aggregation of resources in ConfigMaps. It's entirely possible that future versions of Helm could adopt the Application kind instead of the ConfigMap to track installed applications so that it's not "yet another thing". It may be not be immediately achievable, but I believe this is where we should be heading in the long run.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@smarterclayton to me it looks like the mechanism you are describing is actually an intrinsic part of the solution proposed here.
Modeling the relationship between the objects is a critical part, and this new kind is an addition to retrieve aggregated informations or perform higher level operations - based on this relationship model.

The proposal could at least structure it that way: A definition of how to define relationships, an Application API leveraging this definition in order to provide high-level informations and operations.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that, based on the feedback of this proposal, many users are creating yet another thing to track. The purpose of this proposal is to help us converge, as a community, on the definition of that thing.

standard way to consume application level health checks.

#### Application Update
As an application developer, maintainer, or administrator, I can update the configuration of my application.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not answered or addressed by the current proposal. It says nothing about the "configuration" of an application.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the application update here includes removing existing components and adding new ones ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It refers to the configuration of the application object.

Copy link

@krmayankk krmayankk Feb 11, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right i specifically mean, when the user updates his application object, he may choose to remove some old Deployments and add new ones? Do you include that scenario in this or not ?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the components Deployments are entirely driven by application object, then I believe it is yes as updating application object may re-converge/initiate the Deployments.

HealthCheck* ApplicationHealthCheck `json:"healthCheck" protobuf:"bytes,5,opt,name=healthCheck"`

// Version is an optional version indicator for the application.
Version string `json:"version,omitempty" protobuf:"bytes,6,opt,name=version"`

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why have this and the "Generaton" field?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn’t Generation the version of the object and Version the version of the application? I.e my-app v1.2 has an object with a generation version of 3? (Since this is the 3rd time I updated it)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One of the issues we've had with version in Helm is confusion over what is the application version and what is the version of the set of Kubernetes resource definitions. Often these will deviate (an update to a Kubernetes definition should bump the version even if the application version has not changed). In Helm we introduced the AppVersion field to solve this, should we do the same here?

Version string `json:"version,omitempty" protobuf:"bytes,6,opt,name=version"`

// AboutURL is a URL pointing to any additional information about the application.
AboutURL string `json:"url,omitempty" protobuf:"bytes,7,opt,name=url"`

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this necessary? What user requirement is this satisfying? Are there any examples?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commenting generally on descriptive metadata (not just this AboutURL field), the user requirements have to do with publishing and consuming off-the-shelf applications.

For example, as a new Kubernetes user, if I run "helm install stable/wordpress", I'd like to be able to navigate to my favorite k8s dashboard and find a basic landing page about the instance WordPress that I just created. The grouping and health mechanisms in this proposal begin to satisfy this requirement, but what I'd also like to see on such a page is high-level descriptive information, links to documentation, where to find the Admin URL, username, and password, suggested next steps, and similar app-specific content.

As for examples, I would point to Helm's Chart.yaml and NOTES.txt. These documents represent structured descriptive metadata and lightweight instructions. I see this proposal as an inter-operable standard for that same class of information, with the added benefit that the information would be available for rendering in a UI.

By the way, I think this user requirement could be called out more directly in the "User Stories" section.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps we can avoid needing consensus on which fields are sufficiently ubiquitous (and their semantics) by modeling all fields not relevant to the application controller with @deustis's suggestion below (ordered list of key-value pairs).

This model may be insufficient if multiple UI providers require different semantics of the same Application instance, so we might add a nesting layer to differentiate between UI providers (a mapping of UI provider to ordered list of key-value pairs).

Keywords []string `json:"keywords,omitempty" protobuf:"bytes,10,opt,name=keywords"`

// LicenseURL is a an optional URL pointing to any licensing information for the application.
LicenseURL string `json:"license,omitempty" protobuf:"bytes,11,opt,name=license"`

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

License for what? Your example has multiple pieces of software each with its own license. So it clearly doesn't make sense for this to have, say, the license of Wordpress here. Are you suggesting that the YAML files themselves are under license? Or that the configuration is licensed?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Many tools will let you put in an SPDX identifiers or a url to a license. Consider that?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another way to ask what/why: If this didn't exist initially, what would that block?


// HealthCheck is an optional application level health check.
HealthCheck* ApplicationHealthCheck `json:"healthCheck" protobuf:"bytes,5,opt,name=healthCheck"`

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fact that none of the fields below this point in the spec even get used in the examples makes me question whether they are necessary to add to the spec. For the most part, this is the only place these fields are even mentioned in the spec.

## Summary
Kubernetes has many primitives for managing workloads (e.g. Pods, ReplicaSets, Deployments, DaemonSets and
StatefulSets), storage (e.g. PersistentVolumeClaims and PersistentVolumes), and networking (e.g. Services,
Headless Services, and Ingeresses). When these primitives are aggregated to provide a service to an end user or to
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ingresses

inability of UI (User Interface) designers to surface information about applications to users.

To address these issues, we propose to add an Application Kind as a first class citizen to the apps Group of the
Kuberentes API. This Kind can be used by tools to communicate that the applications they create are more than just
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kubernetes

## Motivation
As an example, consider how [WordPress](https://github.com/WordPress/WordPress), a simple CMS (Content Management
System) is created via a manifest. The manifest below creates two StatefulSets (one for a MySQL RDBMs instance and
another for the WordPress web server), a Headless Services for each StatefulSet, a and load balanced Service for the
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/a and/and a/

storage: 250Gi
```

Form an infrastructure management perspective, the manifest above encapsulates everything that is necessary to create
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/Form/From/

// that it has observed.
ObservedGeneration int64 `json:"observedGeneration,omitempty" protobuf:"int64,1,opt,name=observedGeneration"`

// Installed is used by the Application Controller ot report the installed Components and Dependencies of
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/ot/to/


// selector is a label query over kinds that created by the application. It must match the component objects' labels.
// More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/#label-selectors
Selector *metav1.LabelSelector `json:"selector" protobuf:"bytes,4,opt,name=selector"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the application components consist of resources(of the types mentioned in Components) selected by this label?

Version string `json:"version,omitempty" protobuf:"bytes,6,opt,name=version"`

// AboutURL is a URL pointing to any additional information about the application.
AboutURL string `json:"url,omitempty" protobuf:"bytes,7,opt,name=url"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this feels like it could be handled via some well defined annotations which would be more flexible/extensible..does it need to be a first class field?
(i think we're going to have a hard time agreeing on the right set of "application descriptor metadata" so i'd rather see some proposed annotations and let people easily extend/mutate that set of annotations for their tools/uses)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(same comment on most of the other fields being proposed here).

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel the proposal does a good job selecting the minimal set of descriptor metadata. Fields like "version" and "description" at least have been proposed multiple times and probably do stand a chance of getting agreement. The benefit of having some first-class descriptor would be significant, especially UIs to display basic information about off-the-shelf apps (e.g. from Helm).

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would suggest grouping descriptive fields (i.e. intended for human consumption) under some kind of "Descriptor" object

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree about grouping descriptor fields, disagree that "AboutURL" and "LicenseURL" and such qualify as minimal though.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See Descriptor proposal (https://docs.google.com/document/d/12-Ey2d4RXgZLdz1fEHRG6NXEv97o66vECb4JW0nbfqA/edit) and Labels/Annotations proposal (https://docs.google.com/document/d/1EVy0wRJRm5nogkHl38fNKbFrhERmSL_CLNE4cxcsc_M/edit). We should converge these separate efforts to come up with a minimal set of descriptor metadata.

url:
selector:
matchLables:
app: wordpress
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

correct me if i'm wrong, but w/ this api, the application component is going to select all services w/ a label named "wordpress" (and all statefulsets).

Or the selected resources are additionally filtered by the component names in the components map? (I would expect the latter but I haven't seen it clearly spelled out)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually it looks like this spec also introduces a "comonent" label, so i guess it filters on:
resource=resourcetype
app=appname
component=componentname

to construct the set of app resources. Again would be good to clearly state somewhere how this app resource selection is intended to work.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok no, the component label doesn't appear to be used by this spec.

Which makes me wonder about the purpose of the "components" datastructure. All that seems to actually be used is a label selector + a list of resource groupversionkinds. The actual resource names are not used by any part of the spec that I saw, so i don't know what purpose they serve in the data structure.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a better explanation in the CRD type definition. Getting things by name might work if we are okay with every instance of an application mapping distinct names to the GV of the object.

the Application, that match the Application's `Spec.Selector`.
1. For each selected object, validate that the object has a `kubernetes.io/application` that matches the name of the
Application. Discard any objects that do not have such an annotation. This ensures that selector overlap does not
violate the intentions of the application creators and administrators.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so the "component" label is not used... not sure what the point of listing all the components in spec.components is, then. you really only need to list the resource types. (but i do think it would be better to explicitly select resources by a name, not just by type+label).

method should be analogous to that of ReplicaSet and Deployment.
1. Create a new Pod based on the `Template`.
1. The Application Controller shall delete and recreate HealthCheck Pods upon mutation of `Spec.HealthCheck` field of
the application.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

requiring an additional pod definition+resource consumption to do healthchecks seems excessive to me.
Also nothing is going to ensure this pod keeps running, so it should be a replicaset (count=1) imho, if this general mechanism remains.

@mattfarina
Copy link
Contributor

This proposal has a lot to it including metadata, controllers, etc. For a change to core this level of top down detail is needed along with hearty debate. But, if we want to start this as a CRD experiment should we move away from the top down design document to a repo, a handful of people close to the problem and willing to contribute to work on it, and start there?

If this PR is merged with all the details it has and an experiment is started there will likely be a quick divergence from what's put forth here and what's built.

Any suggestions on how to proceed?

@errordeveloper
Copy link
Member

errordeveloper commented Feb 7, 2018

What if we started by proposing a simple structure that is basically a set of pointers to resources, which can be used to:

a) store app metadata
b) find all resources that belong to one app instance
c) delete all resources that belong to one app
d) obtain app status (simply based on the status of all pods that are created by controllers in the set)
e) collect events (and logs) from all resources, controllers and pods in the set

It's very hard to decide about more advanced kind of health check that is proposed here could be a separate CRD that we can discuss separately.
A lot of fields (description, license, maintainers, ...etc) that are suggested here also debatable, I cannot see why those cannot be set as annotations with conventional names, as most seem rather optional.

@mattfarina
Copy link
Contributor

mattfarina commented Feb 8, 2018

@errordeveloper What is proposed seems simple on the surface. But, in practice it may not be.

For example, what if two apps have a set of shared resources? Some folks may describe things in that manner (they do in practice... I've seen it). So the "c) delete all resources that belong to one app" may break a second app. Or, you try to take all of this into account and the logic gets quite complicated.

This kind of opinion is something I might find in a PaaS (but even things like cloud foundry don't delete all the things when an app is deleted because it's messy). K8s is explicitly not a PaaS. If you read what is kubernetes there is a section on what it isn't that starts out:

Kubernetes is not a traditional, all-inclusive PaaS (Platform as a Service) system. It preserves user choice where it is important.

As far as I can tell, this sentiment is still true.

The expectation is that PaaS like features will be built in other projects that sit on top of kubernetes.

It's worth noting that in the comments there is disagreement from people who've worked on tooling like this and are part of SIG Apps, the sponsoring SIG. The SIG doesn't appear to have consensus. If we used lazy consensus by SIG members as the acceptance criteria, this isn't accepted.

We likely need to have some high bandwidth conversations on this with some relevant parties to come up with a workable direction as a next step. I hope to do some of this at the Helm Summit when a number of people are face to face. I expect more of this to happen in SIG Apps as well.

Uses a Deployment instead of Pod for HealthChecks
Moves Controller based GC for future work
Clarifies CRD
Address general comments
Indicates that general aggregation is a non-goal
Updates acceptance critirea
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jbeda, kow3ns

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 OWNERS Files:

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

@kow3ns
Copy link
Member Author

kow3ns commented Feb 9, 2018

@mattfarina
@jbeda
@prydonius
I've updated the proposal to address many of the outstanding issues and to indicate that we will proceed with a CRD and third-party controller. I'd like to propose that we check this version in as accepted and continue the discussion while we prepare a repository to house our work. If the criteria for acceptance is a SIG level interest for continued discussion and collaboration, I'd say that based on the volume of feedback we have decided we are interested.
I will take responsibility for keeping the KEP in sync with the work we perform. Even if the work is high velocity and we deviate frequently and immediately, it won't serve us well to have nothing that captures the desired goals of the effort and the design of what we build. Keeping a design in sync with a incubating and rapidly developing project can be a burden, but, imo, it is important to document our goals, even as they change, lest we loose sight of them.

@errordeveloper
Copy link
Member

For example, what if two apps have a set of shared resources?

I would consider a simple rule where 2 sets of resources that logically overlap should be considered to be broken out into 3 non-overlapping sets.

I think it'd be really great if we could move ahead with a simple proposal to start with, rather then considering edge cases at this point in time. It's CRD we are discussing and not an upstream API.

This kind of opinion is something I might find in a PaaS

I am fully aware that Kubernetes is not a PaaS.

The expectation is that PaaS like features will be built in other projects that sit on top of Kubernetes.

I thought we are having a discussion about a CRD here, as far as I'm concerned that's technically a separate project.

Also, I understand that you are hitting at that this CRD could be a little PaaS-inspired one, but I don't think it has to be. We don't have to describe multi-layer dependencies of components or anything like that, all I'm advocating for is unit of management for a collection of resources, perhaps calling it ResourceSet (or NamedList?) would help, and it'd be the spirit of the naming convention, although I don't think it'd bare any automation semantics, it'd pure a way of holding a bunch of resources that (for the sake of simplicity) explicitly don't have any logical overlap with other sets.

Alternatively, we could use namespace for this purpose, which is reasonable, although there is no way indicate collective status. Another option would be to use labels, however, there is still an issue with status and unlike with a namespace there is nowhere to hang common metadata in a meaningful way and still no common status indicator, plus kubectl {get|delete} all -l ... doesn't actually cover all the kinds of resources, so one has to know what they are looking for.

proliferation on non-interoperable tools, a non-uniform experience for application developers and users, and an
inability of UI (User Interface) designers to surface information about applications to users.

To address these issues, we propose to add an Application Kind as a first as a Custom Resource Definition (CRD)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: this needs rewording, its no longer a Kind with CRD right ?


## Proposal
In order to address the issues discussed in the [previous section](#motivation) we will introduce the Application Kind
to as a CRD extension the Kubernetes API, add the Application Controller to the Controller Manager, and modify

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Application Controller will no longer be part of the Controller Manager , but a separate controller that users must install , right ?


#### Application Deletion
As a user, I can delete my application, and when I delete my Application, its corresponding components are deleted.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kow3ns what about application rollback, marking an application as completed rollout or succesfuflly deployed. That is an important part of determining if this update was sucessfull or has issues and must be rolled back ?

Components map [string]metav1.GroupVersionKind `json:"components,omitempty" protobuf:"bytes,2,opt,name=components"`

// Dependencies is a list of other Applications that this Application depends on.
Dependencies []sting `json:"dependencies" protobuf:"bytes,3,opt,name=dependencies"`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the design doc should spell out that not all dependencies are expected to be expressed via Dependencies, and that in particular cross-namespace and service-based dependencies need not be.

1. The API server shall ensure that the Applications `Spec.Selector` has not been mutated. This latter condition has
been implemented in many places in the Kubernetes API to prevent unintentional orphaning. This is our purpose here.

#### Generation Incrementation
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we add a manually managed AppVersion as suggested by prydonius , then I think there is less issue with the name ObservedGeneration.

challenges here would be multi-tenancy and namespace isolation. If a higher level object aggregates all Applications
of the same Type, it would have to do so across namespaces. This is not compatible with the existing namespace model
for Kubernetes unless we assume that the higher level object is only accessible to cluster administrators with
permissions to access all namespaces in the cluster.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The term App implies something that can be run.
The License, AppVersion, and DashboardURL attributes only make sense to me for a thing that has image(s) and runs.
A common UX for Apps will be to display aggregate resource usage for the pods, resource usage, image versions, and so on.

Although this could be used for non-Podful resource collections, it does dilute the value of the App concept in UX to do so.

Suggest for alpha calling this App, and recommending that it be used for collections that include Pods.

for Kubernetes unless we assume that the higher level object is only accessible to cluster administrators with
permissions to access all namespaces in the cluster.

## Future Work
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to aggregate events that come from the same app. Not sure if app controller is the right thing to do that.

Application to the object.

#### Health Status Reporting.
The Application Controller will do the following to update the `Stauts.Ready` ConditionStatus of an Application object.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does this mean? The app is ready to take full load as a service? The App is ready to use by things that depend on it?

1. Provide a standard way for applications to surface a basic health check to the UIs.
1. Provide an explicit mechanism for applications to declare dependencies on another application.
1. Promote interoperability among ecosystem tools and UIs by creating a standard that tools MAY implement.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is very useful to be able to aggregate things across many pods, such as resource usage/request/limits, log lines, monitoring metrics, and so on. This works best when the pods can be grouped into disjoint sets that are smaller than namespace sized. I wonder if you want to call out disjoint grouping of pods and other resources as an explicit goal. I think the application labeling of resources provides this, but if it is a goal, we'll remember to keep it that way.

In order to achieve this users should do the following.

1. Create an Application object that contains the objects that will be adopted in the Application's `Spec.Components`.
1. Annotate the objects with a `kubernetes.io/application` label that references the Application.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this recursive? Meaning, if I mention a Deployment as a Component, and the Deployment makes and RS that makes a pod, does the pod have the label too? What about a SparkJob, where there is no visible pod template in the parent resource?

Also, "Annotate ... with ... label" makes it unclear if you are talking about a metadata.labels or a metadata.annotations.

last-updated: 2018-01-15
status: accepted
see-also:
[labeling and annotation conventions](https://docs.google.com/document/d/1EVy0wRJRm5nogkHl38fNKbFrhERmSL_CLNE4cxcsc_M)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

@apelisse apelisse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it's just me. But I feel like there are many "links" between components that might make maintenance of these objects more complicated, with very little chance for the owner to understand what is wrong.

// spec.selector to select the resources that match from the components GroupKind at the client's desired version.
type ApplicationSpec struct {
// Type is the type of the application (e.g. WordPress, MySQL, Cassandra).
Type string `json:"type" protobuf:"bytes,1,name=type"`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How is the type different from the name (in the metadata)? In your example below, they both have the same value, what's the purpose? When do you expect them to be different, etc?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on the examples, it seems like the field is intended to be descriptive (not programatically consumed) and captures something about the source of the application instance (open source project, product name, etc.). However, "Type" seems to hint at having a programmatic meaning. If the intent is descriptive, consider alternatives:

  • Title
  • Brand

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there an explicit usage of this field? As there is no clear definition of "type" and user could use it arbitrarily which makes it more like a metadata.


// Selector is a label query over kinds that created by the application. It must match the component objects' labels.
// More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/#label-selectors
Selector *metav1.LabelSelector `json:"selector" protobuf:"bytes,4,opt,name=selector"`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might have missed the explanation somewhere in the document, but why the extra-redundancy here? Is the author supposed to make sure that the components have these selectors? what happens if it doesn't?

If this isn't pure redundancy, why should I include some items in components and target some others with the selector?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.. and what resource types are applying this labelselector to?

The components of the WordPress application are made explicit by the `Spec.Components` of the `word-press` Application.
When clients examine the `Spec.Components` of the Application, they have enough information to retrieve and display
any of the Kubernetes primitives that compose the Application. When any of those components is examined, clients can use
the name indicated by the `kubernetes.io/application` to retrieve the parent Application object.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand the point of this piece of information, but can we avoid the extra-redundancy? I see below that clients should discard objects that don't have this annotation. How do they learn about this error in their configuration file? Beside conflicts (in the case of two apps trying to own the same object), what's preventing the application controller from adding the annotation?

// namespace. The values in the Components map are corresponding GroupKinds of the Application's components. These
// may be used, in conjunction with the Selector, to retrieve an Application's components at the client's desired
// version.
Components map [string]metav1.GroupKind `json:"components,omitempty" protobuf:"bytes,2,opt,name=components"`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Further: why are the keys/names useful at all?

The keys of the Components map are the identifiers of the components. These may not be the names of the components.

.. makes it sound like the identifiers can't usefully be used for anything other than separating the values in this Components list.

Edit: oh wait - this is explained further in the text below. I'm super confused why Components/Selector is encoded in this way. Why isn't an Application just a list of v1.ObjectReferences?


### Service Based Health Checks
Another approach to health checks would be use the number of available endpoints on a Service associated with the
application. We believe that many applications would benefit from such a Service based health check. When this feature

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not clear what this means and how applications will benefit


// Installed is used by the Application Controller to report the installed Components and Dependencies of
// an Application.
Installed []string `json:"installed,omitempty" protobuf:"bytes,2,opt,name=installed"`

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you clarify if the strings are the components name ?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

installed Components and Dependencies seem to be two different things. Are Dependencies here being installed within the application? or they could be external dependencies?

// spec.selector to select the resources that match from the components GroupKind at the client's desired version.
type ApplicationSpec struct {
// Type is the type of the application (e.g. WordPress, MySQL, Cassandra).
Type string `json:"type" protobuf:"bytes,1,name=type"`
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on the examples, it seems like the field is intended to be descriptive (not programatically consumed) and captures something about the source of the application instance (open source project, product name, etc.). However, "Type" seems to hint at having a programmatic meaning. If the intent is descriptive, consider alternatives:

  • Title
  • Brand

// Info is a map of human readable key value pairs. It is used to store metadata pertaining to the application
// where such metadata is not appropriate for a label (i.e. it is not selectable) and requires disambiguation from
// annotations.
Info map[string]string `json:"info,omitempty" protobuf:"bytes,11,opt,name=info"`
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ordering seems important here. As the the author on an application I would assume that UIs will render these pairs in a table. I probably want to make editorial decisions about which info shows up first in that table. Suggest an array of label value pairs instead?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is an example of the intended usage for this field? It sounds exactly like annotations, and I don't understand the "requires disambiguation" comment ... (we can't use annotation namespaces for some reason?)

// Description is a brief string description of the application.
Description string `json:"description,omitempty" protobuf:"bytes,7,opt,name=description"`

// Maintainers is an optional list of maintainers of the application. The maintainers in this list are maintain the
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo: "maintainers in this list are maintain"

Version string `json:"version,omitempty" protobuf:"bytes,6,opt,name=version"`

// Description is a brief string description of the application.
Description string `json:"description,omitempty" protobuf:"bytes,7,opt,name=description"`
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Many of the fields in ApplicationSpec are intended to be human-readable strings. Consider grouping them under one object (e.g "Descriptor").

I think this would clarify the purpose of these fields, particularly for users who are only seeing the API object and not the "human readable" comments in this spec.


// Links is a map of human readable descriptions to URLs containing data about the application. This field can be
// used to include URLs for licenses, dashboards, documentation, or other data pertaining to the application.
Links map[string] string `json:"links,omitempty" protobuf:"bytes,11,opt,name=links"`
Copy link

@deustis deustis Feb 26, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd suggest a list of Link objects, for a couple of reasons:

  • preserve ordering
  • extending metadata on the Link. E.g. categorization (support, docs, licenses, dashboard), content type (link vs age vs embedded video), perhaps also a short description that is longer than the anchor text.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some sort of "name" or "key" field will also be useful for programatically identifying and updating the links. For example, if one of the links is an elected master, it might be updated over time by an Operator.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.. and as Brian pointed out above, we should prefer named lists over key/value objects.

annotations, but, as each tool uses a different scheme, an application created by one tool is generally not recognizable
by another, and UI designers have no standard way to recognize and display an application created by any tool.

Moreover, their is no way to determine if the application is healthy. Even if a UI is able to determine the components
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does "is healthy" mean? How can we usefully summarise the status of a complex application for all consumers with such a naive concept?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe in general there is a need to summarize the status of an "application" instead of its individual components at low level. It could be flexible enough so users can define how they want to summarize it, but I'd like to see how the proposal addresses it concretely.

applications.

#### Application Health
As an application developer, I can surface a basic health check for UIs to consume, and, as a UI developer, I have a
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Weave DaemonSet is an excellent example of why a global "is healthy" boolean is not very useful:

  • If a single host is offline (because hardware failure), does that mean "weave" is unhealthy? (no, node outages are expected and normal)
  • What about if a node is booting up and the kubelet exists, but a single weave pod is still being pulled / hasn't started answering readinessProbes yet? Is "weave" unhealthy? (no, again individual weave pod lifecycle transitions are expected and normal)
  • If most of the weave daemonset pods are offline, is it unhealthy? (perhaps, but this could also be something unrelated like a power outage - and the remaining weave nodes are probably functioning just fine)


// Ready is used to report the readiness of an Application. If an Application contains an ApplicationHealthCheck,
// this field corresponds directly to the Readiness of the Pod generated from that health check. If the
// Application does not contain an ApplicationHealthCheck, this field is set to Unknown.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.. so this field is either a slightly lagged copy of another resource's status field, or "Unknown"? I propose we don't do this and instead just go look at that other resource's status field :P

// namespace. The values in the Components map are corresponding GroupKinds of the Application's components. These
// may be used, in conjunction with the Selector, to retrieve an Application's components at the client's desired
// version.
Components map [string]metav1.GroupKind `json:"components,omitempty" protobuf:"bytes,2,opt,name=components"`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Further: why are the keys/names useful at all?

The keys of the Components map are the identifiers of the components. These may not be the names of the components.

.. makes it sound like the identifiers can't usefully be used for anything other than separating the values in this Components list.

Edit: oh wait - this is explained further in the text below. I'm super confused why Components/Selector is encoded in this way. Why isn't an Application just a list of v1.ObjectReferences?

Selector *metav1.LabelSelector `json:"selector" protobuf:"bytes,4,opt,name=selector"`

// HealthCheck is the name of an optional Deployment that provides the health status of the Application. When the
// Deployment is healthy, the application is considered to be healthy.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think "healthy" is a well defined concept for a Deployment. Do you mean when the Deployment has condition Available=True?


// Selector is a label query over kinds that created by the application. It must match the component objects' labels.
// More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/#label-selectors
Selector *metav1.LabelSelector `json:"selector" protobuf:"bytes,4,opt,name=selector"`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.. and what resource types are applying this labelselector to?



#### Namespaces
All components of an Application must reside in the same namespace as the Application. Given the design of Kubernetes
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So does this mean that "Applications" can't include ClusterRoles and bindings, CRDs, etc? How would (eg) nginx-ingress be represented in this proposal?

1. For each selected object, validate that the object has a `kubernetes.io/application` that matches the name of the
Application. Discard any objects that do not have such an annotation. This ensures that selector overlap does not
violate the intentions of the application creators and administrators.
1. Unless otherwise indicated, clients should ensure that the `OwnerReferences` of the object contain the `UID` of the
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OwnerReferences only work within the same namespace fwiw. So we can't (eg) link a ClusterRole to the Application that defined it.

1. If no such Application is found, the Application is either deleted or has not yet been created.
1. If the Application is found, validate that the `Spec.Selector` of the Application object matches the labels of the
object. If the object's labels do not match, the object has been orphaned.
1. Check the `OwnerReferences` of the object for a reference to the Application by `UID` to ensure that ownership has
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what happens if this ownerref check fails? In particular, if the OwnerRef check is necessary, why isn't it also sufficient (and we can drop the ../application annotation)?

1. For each name in the `Spec.Dependencies` list, retrieve the Application of that name, in the namespace of the
Application under consideration. If no such Application is found, the dependency is not installed.

#### Health Checks
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is my favorite feature of the entire proposal.

This is my least favourite feature of the entire proposal. ;)

In particular: I don't believe application "health" for every consumer can be summarised by a single value in a way that is at all useful. Imo, a future system filled with dozens of these contrived "toy" healthchecks would be worse than a system with none of them, or one with a complex, functional monitoring solution.

"do not actually try to run this pod unless this other set of resources I provide is passing health checks"

This is a terrible idea and a retroactive step for k8s imo.

"Health" is a level-triggered thing, not a state transition - an application can go in and out of "healthy" multiple times over its lifetime with no warning. What does that mean for "do not run this pod unless ..."? Should we cancel the pod and re-run it every time the health status changes? Abort the pod immediately on "unhealthy"? What about when the app has started failing, but not enough times to be marked as "unhealthy" yet?

Because of these ambiguities, the k8s pattern has deliberately been "try, and retry on failure" - and "wait until dependency is healthy, then assume everything will magically succeed forever" as suggested is a strong anti-pattern.

More importantly: What happens when the controller responsible fails to notice a A->B->A transition for whatever reason? Every other controller has been designed to be level triggered and to eventually converge, and the api and watch mechanisms expect this. Making it required that this controller notice every unhealthy<->healthy edge transition is going to be extremely fragile and hard to implement.

(I know the above is unusually severe. I'm trying to communicate how strongly I disagree with adding healthchecking as part of this proposal, and no direct personal criticism or offence is intended)

(e.g All Services are created and all Deployments are fully replicated), this does not necessarily imply that the
application as a whole is able to service requests.

### Goals
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems to me that this Application construct is mainly for grouping/aggregating resources in K8s to be a manageable higher level unit. Do we consider to cover other lifecycle aspects of an application like multiple environments, remediation, CI/CD?

standard way to consume application level health checks.

#### Application Update
As an application developer, maintainer, or administrator, I can update the configuration of my application.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the components Deployments are entirely driven by application object, then I believe it is yes as updating application object may re-converge/initiate the Deployments.


// Installed is used by the Application Controller to report the installed Components and Dependencies of
// an Application.
Installed []string `json:"installed,omitempty" protobuf:"bytes,2,opt,name=installed"`
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

installed Components and Dependencies seem to be two different things. Are Dependencies here being installed within the application? or they could be external dependencies?

// spec.selector to select the resources that match from the components GroupKind at the client's desired version.
type ApplicationSpec struct {
// Type is the type of the application (e.g. WordPress, MySQL, Cassandra).
Type string `json:"type" protobuf:"bytes,1,name=type"`
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there an explicit usage of this field? As there is no clear definition of "type" and user could use it arbitrarily which makes it more like a metadata.

Components map [string]metav1.GroupKind `json:"components,omitempty" protobuf:"bytes,2,opt,name=components"`

// Dependencies is a list of other Applications that this Application depends on.
Dependencies []sting `json:"dependencies" protobuf:"bytes,3,opt,name=dependencies"`
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is supposed to capture external dependencies, you might want to allow dependency which is not yet modeled as an application, for example, Service, to make bootstrapping easier.

challenges here would be multi-tenancy and namespace isolation. If a higher level object aggregates all Applications
of the same Type, it would have to do so across namespaces. This is not compatible with the existing namespace model
for Kubernetes unless we assume that the higher level object is only accessible to cluster administrators with
permissions to access all namespaces in the cluster.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@erictune there are some concepts here not naturally tying to ResourceCollection like dependencies but more an application level thing. ResourceCollection in your example can simply be done by using labels.

storage: 250Gi
```

The components of the WordPress application are made explicit by the `Spec.Components` of the `word-press` Application.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

word-press -> wordpress?

@mattfarina
Copy link
Contributor

@kow3ns since this is now being worked on at a sub-project and CRD, should this be closed.

For future reference, part of the reason activity stopped here is that the length caused GitHub to crash when trying to view it. It appears that problem has been fixed.

@jdumars
Copy link
Member

jdumars commented Jun 25, 2018

@kow3ns thoughts on @mattfarina comment above about closing this?

@bgrant0607
Copy link
Member

I'm closing.

In the future we need to figure out how to get a MVP merged in a timely fashion.

https://github.com/kubernetes-sigs/application

@bgrant0607 bgrant0607 closed this Aug 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. sig/apps Categorizes an issue or PR as relevant to SIG Apps. sig/architecture Categorizes an issue or PR as relevant to SIG Architecture. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.