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

Adding documentation on recommended labels and annotations #8703

Merged
merged 3 commits into from
Jul 9, 2018

Conversation

mattfarina
Copy link
Contributor

This is a continuation of the PR at #7695 that was closed due to structure changes. This includes those changes as well as some feedback.

cc: @heckj @garethr @prydonius @errordeveloper @bgrant0607 @pwittrock @dlorenc @ant31 @erictune @jbeda @demobox @chenopis @Bradamant3 @zacharysarah

This content was developed as part of the App Def WG
@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels May 23, 2018
@k8sio-netlify-preview-bot
Copy link
Collaborator

k8sio-netlify-preview-bot commented May 23, 2018

Deploy preview for kubernetes-io-master-staging ready!

Built with commit 4294dda

https://deploy-preview-8703--kubernetes-io-master-staging.netlify.com

| `app.kubernetes.io/part-of` | The name of a higher level application this one is part of | `wordpress` | string |
| `app.kubernetes.io/managed-by` | The tool being used to manage the operation of an application | `helm` | string |

To illustrative these labels in action consider the following StatefulSet object
Copy link
Contributor

Choose a reason for hiding this comment

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

typo: illustrate

Copy link
Contributor

Choose a reason for hiding this comment

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

Replace lines 47-48 with:

To illustrate these labels in action, consider the following StatefulSet object:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

| `app.kubernetes.io/version` | The current version of the application (e.g., a semantic version, revision hash, etc.) | `5.7.21` | string |
| `app.kubernetes.io/component` | The component within the architecture | `database` | string |
| `app.kubernetes.io/part-of` | The name of a higher level application this one is part of | `wordpress` | string |
| `app.kubernetes.io/managed-by` | The tool being used to manage the operation of an application | `helm` | string |
Copy link
Contributor

Choose a reason for hiding this comment

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

I think there was still some discussion in the other PR on whether or not managed-by should be a label or annotation. Did that discussion get resolved out of band somewhere?

Copy link
Member

Choose a reason for hiding this comment

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

The separation between label/annotation was made answering this question:

  • Is a tool or user may want to filter / query by the label XYZ
    If yes = labels
    if no = annotation

I don't know... I tend to say yes for managed-by,

  • Tool: show me all resources I've deployed managed-by: me and associated to app: wordpress

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree with @ant31 on this. Labels can be queried and the tool managing something is worth querying.

used.

## Labels

Copy link
Member

Choose a reason for hiding this comment

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

Need a note about where to apply those labels (everywhere).
e.g.

In order to take full advantage of using those labels, they should be applied similarly on every resources kind.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. I tweaked the language.

```

## Annotations

Copy link
Member

Choose a reason for hiding this comment

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

I'd remove the annotations from the first iteration, it overlaps with the 'ApplicationCRD', which seems a better approach (IMO) to hold metadata around an application (usage/website/maintainers...) so I'd wait a bit more.

We may keep the section as place-holder and explain why we are not proposing any annotation yet and point to the AppCrd repo.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

After the discussion we had in the App Def WG on this I've gone ahead and removed annotations.


| Key | Description | Example | Type |
| ----------------------------------- | --------------------- | -------- | ---- |
| `app.kubernetes.io/name` | The name of the application | `mysql` | string |
Copy link
Member

Choose a reason for hiding this comment

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

was it name or app ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

app.kubernetes.io/app seems redundant, doesn't it? app is in there twice. name says that it's the name of the app.

@fabiand
Copy link
Contributor

fabiand commented May 28, 2018

I actually wonder - In the context of CRDs, did the discussion distinguish between the controller and the actual workloads it manages? I.e. KubeVirt has infra which watches CRs (controller), and launches pods 8workloads) for them.

To which part of those should the labels be applied?
Is it worth to add the specific note about controllers/operators to this doc to help developers decide where to put the labels?
It could actually apply to both parts.

Also: Will the "Application CRD" also be continued to be discussed?

@ant31
Copy link
Member

ant31 commented May 30, 2018

@fabiand this the goal of label 'managed-by'.
So you have one operator-controller installed via kubectl, the controller then create a resources.
The controller/operator deployment would have:

metadata:
  labels:
   app.kubernetes.io/name: cool-operator
   app.kubernetes.io/version: 2.0.0
   app.kubernetes.io/managed-by: kubectl

The resource created by the operator will then be:

metadata:
  labels:
   app.kubernetes.io/name: my-managed-app
   app.kubernetes.io/version: 1.5.4
   app.kubernetes.io/managed-by: cool-operator

Does it answer your use case?

Will the "Application CRD" also be continued to be discussed?

Yes, this proposal doesn't overlap and goals are different.

Copy link
Contributor

@zacharysarah zacharysarah left a comment

Choose a reason for hiding this comment

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

@mattfarina Great start, thanks for re-opening this. ✨ Some edits for tighter language/style consistency.

---

{{% capture overview %}}
Many tools can be used to visualize and manage Kubernetes objects. The tools used
Copy link
Contributor

Choose a reason for hiding this comment

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

Replace this paragraph (lines 7-10) with:

You can visualize and manage Kubernetes objects with more tools than kubectl and the dashboard. A common set of labels and annotation allows tools to work interoperably, describing objects in a common manner that all tools can understand.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

between tools, a common set of labels and annotation allows objects to be described
in a common manner that all tools can understand.

In addition to supporting tooling, the recommended labels and annotations describe
Copy link
Contributor

Choose a reason for hiding this comment

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

Replace lines 12-14 with:

In addition to supporting tooling, the recommended labels and annotations describe applications in a way that can be queried.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

{{% /capture %}}

{{% capture body %}}
The recommended object metadata is broken apart so that some of it is labels while
Copy link
Contributor

Choose a reason for hiding this comment

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

Replaces lines 18-21 with:

The recommended object metadata is broken apart into labels and annotations. Labels are shorter: you can include them when querying the Kubernetes API. Annotations are for longer data that's not meant to be queried.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed this section since we reduced document scope to just labels.

queried.

The metadata is organized around the concept of an Application. Kubernetes is not
a Platform as a Service and doesn't have or enforce a formal notion of an application.
Copy link
Contributor

Choose a reason for hiding this comment

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

Replace:

a Platform as a Service and

with:

a platform as a service (PaaS) and

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

the Kubernetes API. Annotations allow for longer data that's not meant to be
queried.

The metadata is organized around the concept of an Application. Kubernetes is not
Copy link
Contributor

Choose a reason for hiding this comment

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

Replace line 23 with:

The metadata is organized around the concept of an _application_. Kubernetes is not

So:

The metadata is organized around the concept of an application. Kubernetes is not

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Note, these are recommended labels and annotations. They aid the use of managing
applications but are not required for any core tooling to work.

One common theme you'll notice about the labels and annotations is that they
Copy link
Contributor

Choose a reason for hiding this comment

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

Replaces lines 31-35 with:

> Shared labels and annotations share a common prefix: `app.kubernetes.io`. Labels and annotations without a prefix are private to users. The shared prefix ensures that shared labels do not interfere with custom user labels. 

So:

Shared labels and annotations share a common prefix: app.kubernetes.io. Labels and annotations without a prefix are private to users. The shared prefix ensures that shared labels do not interfere with custom user labels.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

| `app.kubernetes.io/part-of` | The name of a higher level application this one is part of | `wordpress` | string |
| `app.kubernetes.io/managed-by` | The tool being used to manage the operation of an application | `helm` | string |

To illustrative these labels in action consider the following StatefulSet object
Copy link
Contributor

Choose a reason for hiding this comment

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

Replace lines 47-48 with:

To illustrate these labels in action, consider the following StatefulSet object:

| `app.kubernetes.io/usage` | The location of an applications usage information (e.g., details for how it's being run by an organization). A preference is given to version specific information | `https://example.com/foo/1.2/` | URL |
| `app.kubernetes.io/website` | A website to find out more information about the application in general | `https://wordpress.org` | URL |

To illustrative these annotations in action consider the following StatefulSet object
Copy link
Contributor

Choose a reason for hiding this comment

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

Replace lines 69-70 with:

To illustrate these annotations in action, consider the following StatefulSet object:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was removed since the annotations were removed all together.

...
```

MySQL is exposed as a `StatefulSet` with metadata for both it and the larger application it is apart of:
Copy link
Contributor

Choose a reason for hiding this comment

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

Replace with:

MySQL is exposed as a `StatefulSet` with metadata for both it and the larger application it belongs to:

So:

MySQL is exposed as a StatefulSet with metadata for both it and the larger application it belongs to:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done


### Web Application With A Database

A slightly more complicated application may be a web application and a database. For this example we consider WordPress using a MySQL database. To add to the metadata complexity Helm is used to install the application. The following snippets illustrate the start of objects used to deploy this application.
Copy link
Contributor

Choose a reason for hiding this comment

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

Replace with:

Consider a slightly more complicated application: a web application (WordPress) using a database (MySQL), installed using Helm. The following snippets illustrate the start of objects used to deploy this application.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@mattfarina
Copy link
Contributor Author

@fabiand The application CRD/controller will continue as discussed.

Everything on the workloads API (deployments, jobs, etc) should have these labels. When it comes to CRDs, it would make sense if they are part of an application.

@mattfarina
Copy link
Contributor Author

@zacharysarah I responded to all of your editorial comments. Please let me know if there is anything else I need to do.

@ant31 @garethr @prydonius @kow3ns can one of you give this a technical review.

Copy link
Member

@ant31 ant31 left a comment

Choose a reason for hiding this comment

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

LGTM - on the technical side.

Copy link
Contributor

@prydonius prydonius left a comment

Choose a reason for hiding this comment

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

LGTM also

@jorgemoralespou
Copy link

How would I select everything that is part of an application? If I were to delete, export all the resources the application wordpress (in the example for application with database), what query would I need to run? Would it need to be multiple queries?

kubecetl get deployment,svc,... -l=app.kubernetes.io/name:wordpress 

Would leave the database in the example (mysql) out.

@ant31
Copy link
Member

ant31 commented Jun 18, 2018

@jorgemoralespou it would be:

kubecetl get deployment,svc,... -l=app.kubernetes.io/part-of:wordpress 

@jorgemoralespou
Copy link

@ant31 then maybe the examples need to be updated. If you look into the examples already documented, those labels are not on the Deployment and the service for "wordpress" so that query would not select them.

@ant31
Copy link
Member

ant31 commented Jun 18, 2018

@jorgemoralespou can you comment on the PR where it should be added ?

Is adding a section with query exampels could help ?

1. List all resources related to wordpress:
$ kubectl get all -l=app.kubernetes.io/part-of=wordpress 

2. List all resources associated to the mysql database of wordpress 
$ kubectl get all -l app.kubernetes.io/part-of=wordpress,app.kubernetes.io/name=mysql

3. List all resources deployed by the operator/tool 
$ kubectl get all -l app.kubernetes.io/managed-by=olm

4. Find all stateful/database resources
$ kubectl get all -l app.kubernetes.io/component=database

@jorgemoralespou
Copy link

@ant31 A section with queries would definitely help, but my concern is more with a flaw in the design you're laying out, than a flaw in documentation.
In my team, we've been using similar conventions for more than a year already and the problem I see is two:

  • since some of the label are optional, it's difficult to rely on them
  • there's not a one label that can select the "whole" application. In the examples provided for web application with a database there are 2 resources that belongs to component "wordpress" part of application "wordpress" and there's also 2 resources (StatefulSet, Service) that are also part of "application=wordpress" but are component mysql.

With any of the queries you've expressed before I can either get part of the resources in the query or more resources than I would possibly want.
Mysql is a component of wordpress. Wordpress is the application. So, in order for me to select both at the same time, what would be my complete "wordpress" application, I would need two queries or a query that could have some flaws (possibly selecting something else).

I hope this explains my concern.

In our use of labeling for apps/components, we always label every component with app=NAME_OF_APP and then every component with component=NAME_OF_COMPONENT so we can either select one or all with one single query.

@ant31
Copy link
Member

ant31 commented Jun 18, 2018

since some of the label are optional, it's difficult to rely on them

In the documentation, none of them are marked as optional. It's still an opt-in for each tool, an organization to follow them or not.
As such, upstream tooling won't highly rely on them but may implement so shortcut or UI assuming they 'might' be present.

there's not a one label that can select the "whole" application.

This is the label app.kubernetes.io/part-of.

  • Get only wordpress backend resources (without the mysql ones):
kubectl get all -l app.kubernetes.io/part-of=wordpress,app.kubernetes.io/name=wordpress
  • Get only the wordpress mysql resources
kubectl get all -l app.kubernetes.io/part-of=wordpress,app.kubernetes.io/name=mysql
  • Get ALL wordpress resources (database + backend):
kubectl get all -l app.kubernetes.io/part-of=wordpress

What is missing ?

@jorgemoralespou
Copy link

@ant31 In the document example, the wordpress core component is defined as this:

apiVersion: apps/v1
kind: Deployment
metadata:
  labels:
    app.kubernetes.io/name: wordpress
    app.kubernetes.io/version: "4.9.4"
    app.kubernetes.io/managed-by: helm
apiVersion: v1
kind: Service
metadata:
  labels:
    app.kubernetes.io/name: wordpress
    app.kubernetes.io/version: "4.9.4"
    app.kubernetes.io/managed-by: helm

So, given your answer:

Get ALL wordpress resources (database + backend):
kubectl get all -l app.kubernetes.io/part-of=wordpress

Will not select these resources. If the labels are not optional (I might have that from the previous PR that lead to this one) and are expected on every resource, all of them, then please use all the labels in all the examples, as otherwise will be confusing. If the labels are not mandatory for every resource, then document minimal set of labels to properly identify every piece in a multi component application.

@mattfarina
Copy link
Contributor Author

@jorgemoralespou I think a few things are important about this:

  1. The primary use of these is not people using kubectl. The goal is to build higher level tools and abstractions and have metadata to support those in a cross tool compatible manner.
  2. There is a different between an app and an instance of an app. If you install wordpress twice in the same namespace there is not way, from this, to query just one of them. That's intentional because...
  3. This is meant to compliment the application CRD work going on elsewhere. That will deal with grouping the resources for an application.

It would be nice to have one release grouping name for an instance of an application that all assets can have as a label. The reason I'm not pushing for that, yet, is because I would like to work out how to couple that to the application CRD as the recommended name to use. Then the circle can be complete.

If we do it outside the application CRD we will have other problems to solve such as name uniqueness and tool interoperability around handing/generating unique names.

Can we put off the unique label until the application CRD is beta and ready for people to start using it?

@fabiand
Copy link
Contributor

fabiand commented Jun 18, 2018

Does it answer your use case?

Yes, thanks! @ant31 @mattfarina

@jorgemoralespou
Copy link

@mattfarina thanks for your reply, but you got me even more confused.

When you say:

The primary use of these is not people using kubectl. The goal is to build higher level tools and abstractions and have metadata to support those in a cross tool compatible manner.

I've always thought that kubernetes provided two different "identifying" aspects to the resources. On one side, labels, which per the documentation are "Labels are intended to be used to specify identifying attributes of objects that are meaningful and relevant to users, but do not directly imply semantics to the core system", and then annotations, that are also documented as "annotations to attach arbitrary non-identifying metadata to objects. Clients such as tools and libraries can retrieve this metadata". So, not sure how this match with your statement above.

Then you continue with:

There is a different between an app and an instance of an app. If you install wordpress twice in the same namespace there is not way, from this, to query just one of them. That's intentional because...

No matter how many times I read this document/proposal, I can not really glimpse this statement you mention. If this is the intent, this should be clearly documented to prevent misunderstanding.

And then you say:

This is meant to compliment the application CRD work going on elsewhere. That will deal with grouping the resources for an application.

Reading that github repo's documentation, I'm even more lost, as even though it defines an application CRD, then it uses the labels here proposed as selector. So, if I combine both examples then things would not work, as in the CRD you identify an instance of an application using the label "app.kubernetes.io/name" while here this label has different values as it does identify the component name (part of an application).

Maybe I'm overcomplicating things, but as they are documented, both proposals can not work together IMHO.

Just take my comments as a casual user, I'm in no way a k8s developer, and you guys here probably know much more and better. I hope not to be making noise, but to help make things consistent, as this is key for us, where we consider apps a fundamental concept, as we're developers.

Cheers,

@ant31
Copy link
Member

ant31 commented Jun 21, 2018

@jorgemoralespou thank you for your feedback.

We're going to discuss this PR during next sig-apps meeting (Monday 25th June, 6pm CET).
Some of your points are valid and in fact, we'll discuss an additional label app.kubernetes.io/instance.

"Labels are intended to be used to specify identifying attributes of objects that are meaningful and relevant to users, but do not directly imply semantics to the core system

Most controllers are based on matching labels, the Label entity is core to kubernetes concepts. Nearly nothing is working without using labels. But no label key/value is hard-coded or required, it's all based on matchSelector.
If we look at the ecosystem, users are using different keys with the same semantics:
e.g: [app, name, k8s-app, k8-name, app-name ].
The goal of this recommendation is to agree on most common and useful labels already used by the community. With this convention, tools and UI can softly rely on them to implement better UX.
In short:

  1. As a Human, when I open a file, all the small differences don't really matter, I understand the intention and meaning. I know that app and k8s-app labels mean the same.
  2. For a tool, it's a different story. In example, If I want to create a 'nice' UI that displays the application name and version using a larger font, how do I do? How do I know which label to pick?

This why this recommendation is toward tools and not users.

Reading that github repo's documentation, I'm even more lost, as even though it defines an application CRD, then it uses the labels here proposed as selector

The AppCRD is using matchLabels, it's possible to use any label. The CRD doesn't default nor force to use the labels recommended here.
It's a new resource that represents an instance of an application and holds its metadata (description, maintainers....). It will be a better answer to some of your concerns about grouping application resources and its dependencies.

@jorgemoralespou
Copy link

@ant31 I don't want to sound like a troll, I just keep discussing this to try to make sure it'll solve what you intend. Also because I can't attend sig-apps, I'll be on the air. Anyways.
I totally understand what is the intent, and I totally agree that standardizing labels is critical, I just not sure that the labels, as you're defining them will help, as in my opinion, for this (or the CRD) all the components of an app need to have a unique (and the only mandatory, or as mandatory as it could be) label that identifies the app it belongs to. Whether that label is app.kubernetes.io/part-ofor app.kubernetes.io/name does not really matter, as long as all the resources within an app can be selected at once. The CRD will need to select them. The tools will need to select them.

My point, and why I complained, is that in the examples you provide, there's no way I can select with just one single selector all the components of an app. And as such, the CRD as it uses a single LabelSelector it will not be able to model my app, and I will need to include an extra label, that I can set on all the resources (not the ones you define here) to be able to create my app resource.

Taking the example from the CRD, current implementation, and the examples in this proposal:

apiVersion: app.k8s.io/v1alpha1
kind: Application
metadata:
  name: "wordpress-01"
  labels:
    app.kubernetes.io/name: "wordpress-01"
    app.kubernetes.io/version: "3"
spec:
  type: "wordpress"
  selector:
    matchLabels:
     app.kubernetes.io/name: "wordpress"
  componentKinds:
    - group: core
      kind: Service
    - group: apps
      kind: Deployment
    - group: apps
      kind: StatefulSet
  version: "4.9.4"
  description: "WordPress is open source software you can use to create a beautiful website, blog, or app."

This would select only the wordpress svc and deployment in your example for a web application with a database. There's no way, with the examples you're using that I could create my application CRD.

Could you just provide me with the complete definition of the 4 resources there(mysql svc, mysql statfulset, wordpress svc and wordpress deployment) that could be selected with an application CRD like the above?

Copy link
Contributor

@adambkaplan adambkaplan left a comment

Choose a reason for hiding this comment

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

Suggestion to add labels addressing concerns in PR thread.

labels:
app.kubernetes.io/name: wordpress
app.kubernetes.io/version: "4.9.4"
app.kubernetes.io/managed-by: helm
Copy link
Contributor

Choose a reason for hiding this comment

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

Recommend adding labels:
app.kubernetes.io/component: server
app.kubernetes.io/part-of: wordpress

labels:
app.kubernetes.io/name: wordpress
app.kubernetes.io/version: "4.9.4"
app.kubernetes.io/managed-by: helm
Copy link
Contributor

Choose a reason for hiding this comment

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

Recommend adding labels:
app.kubernetes.io/component: server
app.kubernetes.io/part-of: wordpress

@ant31
Copy link
Member

ant31 commented Jun 22, 2018

My point, and why I complained, is that in the examples you provide, there's no way I can select with just one single selector all the components of an app.

I've already agreed with this point, #8703 (comment), if the examples are not complete, let's update/comment on them

  • Then you said:

but my concern is more with a flaw in the design

then we're going back to the examples are missing some labels:

In the document example, the wordpress core component is defined as this ....

is that in the examples you provide, there's no way I can select with just one single selector all the components of an app
I really think we're looping on the same issue, that we already agreed: we can improve the examples.

So yes, examples could be completed to cover more query usecases.

But even without updating them, we are not enforcing to use any of those labels. User can chose to pick one, 2, 5 or none. It depends on everyone usecase: why would I need 'part-of' If I'm deploying a standalone stateless application ?
What we are suggesting is this documentation is use those labels key if it match what you want to reference: intead of k8s-name uses app.kubernetes.io/name, instead of application-stack uses app.kubernetes.io/part-of.... then tools/ui can look-up for the presence of them and enhance css/display/ux.

To be clear, no problem is being solve here, it's only a consensus around label keys name that are already largely in used by the community. By pushing this convention we are improving the interroperability of application and UX in general.

@mattfarina
Copy link
Contributor Author

@jorgemoralespou I added a section on instance name vs name as well as the instance field we had discussed in SIG Apps. Does this help?

@ant31 @garethr @prydonius can you take a look at the changes?

Copy link
Member

@ant31 ant31 left a comment

Choose a reason for hiding this comment

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

lgtm

@zacharysarah
Copy link
Contributor

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 9, 2018
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: zacharysarah

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:

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 9, 2018
@k8s-ci-robot k8s-ci-robot merged commit c9fe098 into kubernetes:master Jul 9, 2018
@mattfarina mattfarina deleted the labels-annotations2 branch July 16, 2018 18:14
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. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants