Skip to content
This repository has been archived by the owner on Mar 22, 2023. It is now read-only.

Update Kubernetes object labels #779

Merged
merged 4 commits into from
Jun 5, 2019
Merged

Update Kubernetes object labels #779

merged 4 commits into from
Jun 5, 2019

Conversation

DaoWen
Copy link
Contributor

@DaoWen DaoWen commented Jun 5, 2019

Changes proposed in this PR

  • Move service-id->k8s-app-name docstring
  • Add waiter/cluster Kubernetes label
  • Add waiter/service-hash Kubernetes label

Why are we making these changes?

  • Moving the docstring is just refactoring for correct placement before the argument list.
  • Adding the waiter/cluster label helps address Rename waiter-cluster K8s label to waiter/cluster for consistency #721.
  • Adding the waiter/service-hash label will make it easier to search for Kubernetes objects corresponding to a specific Waiter service. Since the existing waiter/service-id is an annotation (due to label length limitations), it can't be used in Kubernetes selectors when searching.

@DaoWen DaoWen requested a review from shamsimam June 5, 2019 13:01
:waiter-cluster cluster-name
:waiter/cluster cluster-name
:waiter/service-hash service-hash
:waiter/user run-as-user}
Copy link
Contributor

Choose a reason for hiding this comment

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

In a separate PR, we should perform a length check on the run-as-user label and trim accordingly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The OS ensures that isn't a problem: user names already have a length limit of 32 chars on posix systems.

https://serverfault.com/questions/294121/what-is-the-maximum-username-length-on-current-gnu-linux-systems

:labels {:app k8s-name
;; TODO - remove waiter-cluster
;; after waiter/cluster is exclusively in use
;; (see GitHub issue #721)
:waiter-cluster cluster-name
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we preserving the waiter-cluster label for backwards compatibility? Or is there a plan to remove it in a subsequent PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We're going to remove it in the near future, but we need to allow time for all the current services to be restarted. We probably won't remove it until after the ReplicaSet → Pod refactoring.

@DaoWen DaoWen changed the title Feature/add k8s labels Update Kubernetes object labels Jun 5, 2019
@DaoWen
Copy link
Contributor Author

DaoWen commented Jun 5, 2019

@shamsimam - RTM

@shamsimam shamsimam merged commit 3a1b1f9 into twosigma:master Jun 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants