[kubernetes-sigs/poseidon] Added chart for firmament-poseidon #4479

nikita15p · 2018-03-27T17:38:32Z

What this PR does / why we need it: The firmament-poseidon chart is used to plug in the Poseidon/Firmament scheduler to the Kubernetes cluster.This scheduler augments the current Kubernetes scheduling capabilities by incorporating a new novel flow network graph based scheduling capabilities alongside the default Kubernetes Scheduler.

k8s-ci-robot · 2018-03-27T17:38:44Z

Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA.

It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.

If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
If you signed the CLA as a corporation, please sign in with your organization's credentials at https://identity.linuxfoundation.org/projects/cncf to be authorized.
If you have done the above and are still having issues with the CLA being reported as unsigned, please email the CNCF helpdesk: helpdesk@rt.linuxfoundation.org

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

nikita15p · 2018-03-27T18:01:15Z

/assign @dhilipkumars

dhilipkumars · 2018-03-27T18:11:04Z

@nikita15p Thanks for the PR, there is a lot needs to be done to get this PR in a good shape. Please have a look at best practices.

Once all the work is done we could remove [WIP] from the titile.

Please sign the Linux Foundation CLA to start with.

nikita15p · 2018-04-02T05:43:20Z

Bump to verify CLA.

Added maintainer details to Chart.yaml

unguiculus · 2018-04-07T09:30:16Z

@nikita15p You created this as a WIP and did not specify any description. With all due respect but I'm not inclined to review this. We get lots of PRs here which is great and keeps us maintainers busy. But next time, please finish you stuff first and then create a PR. If you have questions on how to implement things, please join Slack and ask there before creating a PR. Thank you.

Firmament poseidon charts

dhilipkumars · 2018-06-18T21:51:56Z

/okay-to-test

dhilipkumars · 2018-06-18T21:54:45Z

@kubernetes/charts-maintainers PTAL.

unguiculus

I made an initial review. Please read our review guideliones and apply best practices. Apply my comments throughout the chart.

unguiculus · 2018-06-22T08:33:25Z

stable/firmament-poseidon/templates/_helpers.tpl

+{{- printf "%s-%s" .Release.Name $name | trunc 63 | trimSuffix "-" -}}
+{{- end -}}
+{{- end -}}
+{{- end -}}


The two separarte fullname partials may clash. Also, all partials should have the chart name prefix. how about toing this:

{{- define "firmament-poseidon.firmamentFullname" -}} {{- template "firmament-poseidon.fullname" . -}}-firmament {{- end -}} {{- define "firmament-poseidon.poseidonFullname" -}} {{- template "firmament-poseidon.fullname" . -}}-poseidon {{- end -}

unguiculus · 2018-06-22T08:33:42Z

stable/firmament-poseidon/templates/firmament_deployment.yaml

@@ -0,0 +1,28 @@
+apiVersion: apps/v1beta1


Use apps/v1

unguiculus · 2018-06-22T08:34:26Z

stable/firmament-poseidon/templates/firmament_deployment.yaml

+kind: Deployment
+metadata:
+  name: {{ template "firmament.fullname" .}}
+  namespace: {{ .Values.namespace }}


Why do you need the namespace here? It can be passed to the Helm CLI when installing the chart.

unguiculus · 2018-06-22T08:35:24Z

stable/firmament-poseidon/templates/firmament_deployment.yaml

+  template:
+    metadata:
+      labels:
+        scheduler: {{ .Values.firmament.scheduler }}


Pods should at least have app and release labels.

unguiculus · 2018-06-22T08:36:45Z

stable/firmament-poseidon/templates/firmament_service.yaml

+   scheduler: {{ .Values.firmament.scheduler }}
+spec:
+  selector:
+    scheduler: firmament


While you make the label configurable in the deployment, you hard-code it here. Common practice is to use a component label. Add app and release labels.

unguiculus · 2018-06-22T08:38:05Z

stable/firmament-poseidon/templates/firmament_deployment.yaml

+  namespace: {{ .Values.namespace }}
+  labels:
+    app: {{ template "firmament-poseidon.name" .}}
+    chart: {{ .Chart.Name }}-{{ .Chart.Version }}


Use chart partial everywhere.

unguiculus · 2018-06-22T08:38:24Z

stable/firmament-poseidon/templates/poseidon_serviceaccount.yaml

+apiVersion: v1
+kind: ServiceAccount
+metadata:
+  name: {{.Values.serviceAccount.name}}


You partial from _helpers.tpl

unguiculus · 2018-06-22T08:38:51Z

stable/firmament-poseidon/templates/rbac.yaml

+    chart: {{ .Chart.Name }}-{{ .Chart.Version }}
+    heritage: {{ .Release.Service }}
+    release: {{ .Release.Name }}
+  name: {{ .Values.clusterrolebinding.name }}


Resource names should be based on the fullname parial. Is there a reason why you want this to be configurable?

unguiculus · 2018-06-22T08:39:38Z

stable/firmament-poseidon/values.yaml

+  create: true
+  # The name of the service account to use.
+  # If not set and create is true, a name is generated using the fullname template
+  name: poseidon


Leave default empty

unguiculus · 2018-06-22T08:39:58Z

stable/firmament-poseidon/templates/poseidon_deployment.yaml

+        version: first
+        poseidonservice: {{ .Values.poseidon.service }}
+    spec:
+      serviceAccountName: {{.Values.serviceAccount.name}}


Use partial for service account

dhilipkumars · 2018-06-26T13:58:28Z

@nikita15p Could you please apply all the review comments on this chart?

stale · 2018-08-18T22:46:31Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Any further update will cause the issue/pull request to no longer be considered stale. Thank you for your contributions.

stale · 2018-09-17T23:24:24Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Any further update will cause the issue/pull request to no longer be considered stale. Thank you for your contributions.

k8s-ci-robot · 2018-09-20T12:09:12Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: nikita15p
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: scottrigby

If they are not already assigned, you can assign the PR to them by writing /assign @scottrigby in a comment when ready.

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

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

This reverts commit 98366b19eadaef49a4bde1e8b5a991c0b02a44fc.

stale · 2018-10-21T10:49:12Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Any further update will cause the issue/pull request to no longer be considered stale. Thank you for your contributions.

stale · 2018-11-04T11:20:47Z

This issue is being automatically closed due to inactivity.

Added chart for firmament-poseidon

694bd14

k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 27, 2018

k8s-ci-robot added the cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. label Mar 27, 2018

nikita15p closed this Mar 27, 2018

nikita15p reopened this Mar 27, 2018

k8s-ci-robot assigned dhilipkumars Mar 27, 2018

dhilipkumars changed the title ~~Added chart for firmament-poseidon~~ [WIP] Added chart for firmament-poseidon Mar 27, 2018

k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 27, 2018

dhilipkumars mentioned this pull request Mar 27, 2018

[RFC] Software Release Management Proposal kubernetes-retired/poseidon#8

Closed

Added maintainer details to Chart.yaml

85ed1f4

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Apr 2, 2018

nikita15p and others added 5 commits April 2, 2018 11:43

Merge pull request #1 from nikita15p/firmament-poseidon-charts

bfa3e2a

Added maintainer details to Chart.yaml

Added maintainer details to Chart.yaml

139a662

Merge branch 'master' into firmament-poseidon-charts

03fe3f1

Merge pull request #2 from nikita15p/firmament-poseidon-charts

859b08f

Added maintainer details to Chart.yaml

Added chart for firmament-poseidon

2801b7b

nikita15p and others added 6 commits April 10, 2018 12:35

Added rbac and formatted the yaml files

c95f87a

Merge branch 'master' into firmament-poseidon-charts

9474174

Merge pull request #3 from nikita15p/firmament-poseidon-charts

57ba1e0

Firmament poseidon charts

Updated lint errors

f14d5a0

Update values.yaml

4f604eb

Update _helpers.tpl

76cbf8e

k8s-ci-robot removed the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Apr 16, 2018

k8s-ci-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Apr 16, 2018

dhilipkumars changed the title ~~[WIP] Added chart for firmament-poseidon~~ Added chart for firmament-poseidon Jun 18, 2018

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 18, 2018

k8s-ci-robot removed the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Jun 18, 2018

dhilipkumars changed the title ~~Added chart for firmament-poseidon~~ [kubernetes-sigs/poseidon] Added chart for firmament-poseidon Jun 18, 2018

unguiculus suggested changes Jun 22, 2018

View reviewed changes

mattfarina unassigned dhilipkumars Jul 19, 2018

stale bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Aug 18, 2018

mattfarina added the Contribution Allowed If the contributor has signed the DCO or the CNCF CLA (prior to the move to a DCO). label Aug 27, 2018

stale bot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Aug 27, 2018

stale bot added lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Sep 17, 2018

ey-bot removed the Contribution Allowed If the contributor has signed the DCO or the CNCF CLA (prior to the move to a DCO). label Sep 20, 2018

Added README file

ed04d6a

This reverts commit 98366b19eadaef49a4bde1e8b5a991c0b02a44fc.

nikita15p force-pushed the master branch from af4acc3 to ed04d6a Compare September 21, 2018 10:38

stale bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Oct 21, 2018

stale bot closed this Nov 4, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[kubernetes-sigs/poseidon] Added chart for firmament-poseidon #4479

[kubernetes-sigs/poseidon] Added chart for firmament-poseidon #4479

nikita15p commented Mar 27, 2018 •

edited

Loading

k8s-ci-robot commented Mar 27, 2018

nikita15p commented Mar 27, 2018 •

edited

Loading

dhilipkumars commented Mar 27, 2018 •

edited

Loading

nikita15p commented Apr 2, 2018

unguiculus commented Apr 7, 2018

dhilipkumars commented Jun 18, 2018 •

edited

Loading

dhilipkumars commented Jun 18, 2018

unguiculus left a comment

unguiculus Jun 22, 2018

unguiculus Jun 22, 2018

unguiculus Jun 22, 2018

unguiculus Jun 22, 2018

unguiculus Jun 22, 2018

unguiculus Jun 22, 2018

unguiculus Jun 22, 2018

unguiculus Jun 22, 2018

unguiculus Jun 22, 2018

unguiculus Jun 22, 2018

dhilipkumars commented Jun 26, 2018

stale bot commented Aug 18, 2018

stale bot commented Sep 17, 2018

k8s-ci-robot commented Sep 20, 2018

stale bot commented Oct 21, 2018

stale bot commented Nov 4, 2018

[kubernetes-sigs/poseidon] Added chart for firmament-poseidon #4479

[kubernetes-sigs/poseidon] Added chart for firmament-poseidon #4479

Conversation

nikita15p commented Mar 27, 2018 • edited Loading

k8s-ci-robot commented Mar 27, 2018

nikita15p commented Mar 27, 2018 • edited Loading

dhilipkumars commented Mar 27, 2018 • edited Loading

nikita15p commented Apr 2, 2018

unguiculus commented Apr 7, 2018

dhilipkumars commented Jun 18, 2018 • edited Loading

dhilipkumars commented Jun 18, 2018

unguiculus 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

dhilipkumars commented Jun 26, 2018

stale bot commented Aug 18, 2018

stale bot commented Sep 17, 2018

k8s-ci-robot commented Sep 20, 2018

stale bot commented Oct 21, 2018

stale bot commented Nov 4, 2018

nikita15p commented Mar 27, 2018 •

edited

Loading

nikita15p commented Mar 27, 2018 •

edited

Loading

dhilipkumars commented Mar 27, 2018 •

edited

Loading

dhilipkumars commented Jun 18, 2018 •

edited

Loading