*: Generate all manifests #908

brancz · 2019-10-01T09:14:53Z

What this PR does / why we need it:

Sharding and autosharding introduced a number of manifests that are
very similar, but not identical to the standard manifests. This
introduces generating all manifests using jsonnet and moves them to
the explicit /examples directory.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):

Fixes #903

lilic

We should update the RELEASE.md as well, to include steps to generate the manifests.

lilic · 2019-10-01T09:22:59Z

README.md

@@ -214,7 +214,7 @@ service account token that has read-only access to the Kubernetes cluster.

 #### Kubernetes Deployment

-To deploy this project, you can simply run `kubectl apply -f kubernetes` and a
+To deploy this project, you can simply run `kubectl apply -f kubernetes/standard` and a


standard? Hmm we can come up with a better name I think here.

Any suggestions? I haven't been able to come up with anything better, and this is the standard way to deploy kube-state-metrics.

We said we want to also since we moving it, rename the directory to examples.

lilic · 2019-10-01T09:25:19Z

kubernetes/autosharding/service.yaml

@@ -1,21 +1,19 @@
 apiVersion: v1
 kind: Service
 metadata:
+  labels:
+    app.kubernetes.io/name: kube-state-metrics
+    app.kubernetes.io/version: v1.8.0-rc.0


Should not all be v1.8.0-rc.1?

It's whatever is in the VERSION file

Not true, in the file above its v1.8.0-rc.1.

lilic · 2019-10-01T09:25:33Z

kubernetes/autosharding/statefulset.yaml

@@ -1,53 +1,58 @@
 apiVersion: apps/v1
 kind: StatefulSet
 metadata:
+  labels:
+    app.kubernetes.io/name: kube-state-metrics
+    app.kubernetes.io/version: v1.8.0-rc.0


v1.8.0-rc.1?

lilic · 2019-10-01T09:27:59Z

scripts/jsonnetfile.json

+            "name": "kube-state-metrics",
+            "source": {
+                "local": {
+                    "directory": "../jsonnet/kube-state-metrics"


Where is this directory? 🤔

/jsonnet is in the root of the repo

lilic · 2019-10-01T09:28:20Z

scripts/jsonnetfile.json

+                    "directory": "../jsonnet/kube-state-metrics"
+                }
+            },
+            "version": ""


Should we pin this to master?

it's a directory based package so it will always do whatever is currently in the directory

brancz · 2019-10-01T12:07:41Z

Added a note to the release doc, and I believe I addressed all comments, if you have a suggestion for the name of the "standard" deployment, then I'm happy to adapt it.

lilic

When I run make examples I get some files leftover which should not be generated IMO:

	examples/autosharding/cluster-LRole-LBinding.yaml
	examples/autosharding/cluster-LRole.yaml
	examples/autosharding/role-LBinding.yaml
	examples/autosharding/service-LAccount.yaml
	examples/standard/cluster-LRole-LBinding.yaml
	examples/standard/cluster-LRole.yaml
	examples/standard/service-LAccount.yaml

lilic · 2019-10-01T12:46:15Z

kustomization.yaml

- kubernetes/kube-state-metrics-deployment.yaml
- kubernetes/kube-state-metrics-service-account.yaml
- kubernetes/kube-state-metrics-service.yaml
+- kubernetes/cluster-role-binding.yaml


Should this be examples?

good catch, forgot about this one

lilic · 2019-10-01T12:46:41Z

tests/e2e.sh


 trap finish EXIT

 # set up kube-state-metrics manifests
-kubectl create -f ./kubernetes/kube-state-metrics-service-account.yaml
+kubectl create -f ./kubernetes/standard/service-account.yaml


This should be examples now here as well.

lilic

This works and looks good to me. Will let @tariq1890 do a final 👍

Sharding and autosharding introduced a number of manifests that are very similar, but not identical to the standard manifests. This introduces generating all manifests using jsonnet and moves them to the explicit `/examples` directory.

tariq1890

/lgtm

Thanks @brancz

tariq1890 · 2019-10-01T18:13:24Z

examples/autosharding/cluster-role-binding.yaml

 kind: ClusterRoleBinding
 metadata:
+  labels:
+    app.kubernetes.io/name: kube-state-metrics
+    app.kubernetes.io/version: v1.8.0-rc.1


Thoughts on adding this? Since these are best practices labels, it would be nice to add this to set a good example for others.

Suggested change

app.kubernetes.io/version: v1.8.0-rc.1

app.kubernetes.io/version: v1.8.0-rc.1

app.kubernetes.io/component: monitoring

k8s-ci-robot · 2019-10-02T10:34:35Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: brancz, tariq1890

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [brancz,tariq1890]

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 added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Oct 1, 2019

k8s-ci-robot requested review from andyxning and lilic October 1, 2019 09:15

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 1, 2019

brancz force-pushed the generate branch from b473941 to 20a3a9e Compare October 1, 2019 09:16

lilic suggested changes Oct 1, 2019

View reviewed changes

brancz force-pushed the generate branch 4 times, most recently from f811c00 to 8754347 Compare October 1, 2019 12:06

lilic reviewed Oct 1, 2019

View reviewed changes

brancz force-pushed the generate branch from 8754347 to 7b54085 Compare October 1, 2019 12:47

lilic reviewed Oct 1, 2019

View reviewed changes

brancz force-pushed the generate branch 5 times, most recently from 1734188 to 84202ec Compare October 1, 2019 14:06

lilic reviewed Oct 1, 2019

View reviewed changes

brancz force-pushed the generate branch from 84202ec to 623cbde Compare October 1, 2019 14:52

*: Generate all manifests

994772a

Sharding and autosharding introduced a number of manifests that are very similar, but not identical to the standard manifests. This introduces generating all manifests using jsonnet and moves them to the explicit `/examples` directory.

brancz force-pushed the generate branch from 623cbde to 994772a Compare October 2, 2019 08:40

tariq1890 approved these changes Oct 2, 2019

View reviewed changes

k8s-ci-robot assigned tariq1890 Oct 2, 2019

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 2, 2019

k8s-ci-robot merged commit 51f0b2e into kubernetes:master Oct 2, 2019

brancz deleted the generate branch October 2, 2019 12:00

dudicoco mentioned this pull request Feb 6, 2022

What is the correct readiness probe port? #1678

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

*: Generate all manifests #908

*: Generate all manifests #908

brancz commented Oct 1, 2019 •

edited

Loading

lilic left a comment

lilic Oct 1, 2019

brancz Oct 1, 2019

lilic Oct 1, 2019

lilic Oct 1, 2019

brancz Oct 1, 2019

lilic Oct 1, 2019

lilic Oct 1, 2019

lilic Oct 1, 2019

brancz Oct 1, 2019

lilic Oct 1, 2019

brancz Oct 1, 2019

brancz commented Oct 1, 2019

lilic left a comment

lilic Oct 1, 2019

brancz Oct 1, 2019

lilic Oct 1, 2019

lilic left a comment

tariq1890 left a comment

tariq1890 Oct 1, 2019

k8s-ci-robot commented Oct 2, 2019

	app.kubernetes.io/version: v1.8.0-rc.1
	app.kubernetes.io/version: v1.8.0-rc.1
	app.kubernetes.io/component: monitoring

*: Generate all manifests #908

*: Generate all manifests #908

Conversation

brancz commented Oct 1, 2019 • edited Loading

lilic left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brancz commented Oct 1, 2019

lilic left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lilic left a comment

Choose a reason for hiding this comment

tariq1890 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

k8s-ci-robot commented Oct 2, 2019

brancz commented Oct 1, 2019 •

edited

Loading