Create a K8s app to deploy cncf/devstats for Kubeflow. #91

jlewi · 2018-04-18T18:47:06Z

Create a Docker image with the cncf/devstats binaries
Use a ConfigMap to store the project config
Add stateful sets for influxdb and postgre
Add a stateful set to deploy devstats container so that we can do
kubectl exec to run commands.

/assign @gaocegege
/cc @lukaszgryglicki

Related to #34

This change is

k8s-ci-robot · 2018-04-18T18:47:09Z

@jlewi: GitHub didn't allow me to request PR reviews from the following users: lukaszgryglicki.

Note that only kubeflow members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

Create a Docker image with the cncf/devstats binaries

Use a ConfigMap to store the project config

Add stateful sets for influxdb and postgre

Add a stateful set to deploy devstats container so that we can do
kubectl exec to run commands.

/assign @gaocegege
/cc @lukaszgryglicki

Related to #34

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.

lukaszgryglicki

Approving, added some suggestions/comments.
Please note that I have no ability to run/test this.
So I've just reviewed the code carefully - nothing more.

lukaszgryglicki · 2018-04-18T18:54:57Z

devstats/Dockerfile.devstats

+	git clone https://github.com/cncf/devstats.git devstats
+
+RUN cd ${GOPATH}/src/devstats && \
+	git checkout v0.4.0 && \


This release is old. I would go with master or let me create another v0.5.0 release tomorrow.

lukaszgryglicki · 2018-04-18T18:55:46Z

devstats/Dockerfile.devstats

+
+# Add /go/src/devstats to the path so that all the binaries will be on the path.
+# This is needed by the devstats binary.
+ENV PATH $PATH:/go/src/devstats


All other lgtm for me, but I cannot actually test this and confirm that it works.

No problem; wasn't expecting you to. Thanks for the comments.

lukaszgryglicki · 2018-04-18T18:57:22Z

devstats/Makefile

+# this a ksonnet app and run locally on minikube.
+run_postgre:
+	-docker rm -f postgres
+	docker run -d --name=postgres -p 5432:5432 -e POSTGRES_USER=gha_admin -e POSTGRES_PASSWORD=password -e POSTGRES_DB=gha  postgres:10.1-alpine


Can't test this, so cannot actually say if this os OK or not. Basically it looks OK for me.

lukaszgryglicki · 2018-04-18T18:58:58Z

devstats/README.md

+### Create the config map with the projects file
+
+kubectl create -n devstats configmap projects --from-file=./projects.yaml
+


projects.yaml taken from cncf/devstats repo (it contains 22 or 23 CNCF projects) or just a new projects.yaml containing only kubeflow config?
Basically this file must be customized per your needs.

EDIT: I see that you have created custom projects.yaml: good, lgtm.

lukaszgryglicki · 2018-04-18T19:00:25Z

devstats/README.md

+
+```
+Error(time=2018-04-18 18:39:01.274140856 +0000 UTC):
+Error: 'pq: database "devstats" does not exist'


devstats database is a separate postgres database that only holds logs (gha_logs table).
Its setup is described in INSTALL_*.md files.

Done; thank you.

BTW I'm going to update auto deploy script - to check if that DB exists. If not (it is shared for all projects) assume that this is a "from zero" setup - create database, admin, ro_user, team role etc.

lukaszgryglicki · 2018-04-18T19:02:05Z

devstats/devstats_cli.yaml

+          value: gha
+        - name: PG_PASS
+          value: password
+


You probably need to refer logs database here somehow.
You need two Postgres databases minmally:

Your project's database - can have any name

Logs database named devstats containing single table gha_logs, see INSTALL_*.md files.

I have two databases both stored in a single instance of postgre.

lukaszgryglicki · 2018-04-18T19:03:06Z

devstats/ghadb_postgres.yaml

+        - name: POSTGRES_PASSWORD
+          value: password
+        - name: POSTGRES_DB
+          value: gha


Probably logs database also needed here (or maybe logs database will be in a different pod/yaml/deployment)?

lukaszgryglicki · 2018-04-18T19:05:22Z

devstats/projects.yaml

+    # TODO(jlewi): Move the start date back in time once we are done doing some initial testing
+    start_date: 2018-04-16T00:00:00Z
+    # Can we get rid of join date since we aren't part of CNCF yet?
+    join_date: 2018-04-16T00:00:00Z


You can set it to null just like opencontainers project in CNCF repo:
See: https://github.com/cncf/devstats/blob/master/projects.yaml#L241

gaocegege

Generally LGTM although I am not sure if it works.

Thanks :-)

gaocegege · 2018-04-19T01:52:31Z

devstats/Dockerfile.devstats

+	go get gopkg.in/yaml.v2 && \
+	go get github.com/google/go-github/github && \
+	go get golang.org/x/oauth2 && \
+	go get github.com/mattn/go-sqlite3


BTW, @lukaszgryglicki Could we add vendor in devstats side in the future thus we could avoid go get in the docker image.

Sure we can.

@jlewi https://github.com/cncf/devstats/pull/99 devstas has vendor now and maybe we could not install so many deps in the docker image.

* This deploys postgres in a pod to store the data from github archive and influx db. * There is a cron job to run devstats to regularly sync the data. * There is also a K8s job that can be used to load data from a specific time range (useful for backfilling) * I've verified that data can be loaded into postgres and queried. * I don't know if any data is being dumped to influxdb. * We create a Docker image with the cncf/devstats binaries following the cncf/devstats instructions for installing the binaries in ubuntu 17. * Add stateful sets for influxdb and postgre * Add a stateful set to deploy devstats container so that we can do kubectl exec to run commands.

jlewi · 2018-04-19T05:10:08Z

Thanks PTAL I made significant changes.

gaocegege

Generally LGTM although I am not sure if it works.

gaocegege · 2018-04-19T08:43:03Z

devstats/Dockerfile.devstats

+	# /etc/projects-volume will be volume mounted into the pod.
+	ln -sf /etc/projects-volume/projects.yaml /etc/gha2db/projects.yaml
+
+# TODO(jlewi): Per the instructions for devstats we should increase the number of default connections for postgres


I think we should set the postgre-docker-entrypoint.sh as entrypoint.

The docker image is used to run other things; like the CLI stateful set. Which is why I didn't want to set the entrypoint here and instead do it in the pod spec.

gaocegege · 2018-04-19T08:43:47Z

devstats/Dockerfile.devstats

+
+RUN mkdir -p /home/
+COPY postgre-docker-entrypoint.sh /usr/local/bin/
+RUN chmod a+x /usr/local/bin/postgre-docker-entrypoint.sh


Why do we set the permission in docker image? Could we set it outside and copy it into the image or just set bash /usr/local/bin/postgre-docker-entrypoint.sh as entrypoint?

I don't think permissions are preserved when you copy the file. Usually I have to do chmod a+x to make scripts executable inside the container.

This depends on the number of CPU/cores reported by the go runtime. Our servers report 96 for example. And this is the number of threads that devstats spawn. Sometimes it is too much for postgres default setup. But when the system reports 8 or even 24 cores - you can skip this.

jlewi · 2018-04-19T23:26:52Z

@gaocegege LGTM?

gaocegege · 2018-04-20T02:18:30Z

/lgtm

jlewi · 2018-04-20T06:27:16Z

/approve

k8s-ci-robot · 2018-04-20T06:27:19Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: gaocegege, jlewi

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~~ [jlewi]

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

See kubeflow/kubeflow#1207 Update according to new sections

k8s-ci-robot assigned gaocegege Apr 18, 2018

k8s-ci-robot added the size/L label Apr 18, 2018

lukaszgryglicki approved these changes Apr 18, 2018

View reviewed changes

gaocegege reviewed Apr 19, 2018

View reviewed changes

k8s-ci-robot added size/XXL and removed size/L labels Apr 19, 2018

jlewi force-pushed the devstats branch from 82774a9 to 5c1b3c3 Compare April 19, 2018 05:07

jlewi changed the title ~~Create some K8s manifests to deploy cncf/devstats for Kubeflow.~~ Create a K8s app to deploy cncf/devstats for Kubeflow. Apr 19, 2018

gaocegege mentioned this pull request Apr 19, 2018

[go] Add vendor to make it easy to build Docker image cncf/devstats.archive#98

Closed

gaocegege reviewed Apr 19, 2018

View reviewed changes

Add some instructions about using influx.

3a3172f

jlewi mentioned this pull request Apr 19, 2018

DevStats to track velocity for Kubeflow #34

Closed

k8s-ci-robot added the lgtm label Apr 20, 2018

k8s-ci-robot added the approved label Apr 20, 2018

k8s-ci-robot merged commit ad939bc into kubeflow:master Apr 20, 2018

woop pushed a commit to woop/community that referenced this pull request Nov 16, 2020

Split kubeflow-core into individual prototypes (kubeflow#91)

6cda4b6

See kubeflow/kubeflow#1207 Update according to new sections

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create a K8s app to deploy cncf/devstats for Kubeflow. #91

Create a K8s app to deploy cncf/devstats for Kubeflow. #91

jlewi commented Apr 18, 2018 •

edited

Loading

k8s-ci-robot commented Apr 18, 2018

lukaszgryglicki left a comment

lukaszgryglicki Apr 18, 2018

jlewi Apr 19, 2018

lukaszgryglicki Apr 18, 2018

jlewi Apr 18, 2018

lukaszgryglicki Apr 18, 2018

jlewi Apr 19, 2018

lukaszgryglicki Apr 18, 2018

lukaszgryglicki Apr 18, 2018

jlewi Apr 19, 2018

lukaszgryglicki Apr 19, 2018

lukaszgryglicki Apr 18, 2018

jlewi Apr 19, 2018

lukaszgryglicki Apr 18, 2018

lukaszgryglicki Apr 18, 2018

gaocegege left a comment

gaocegege Apr 19, 2018

lukaszgryglicki Apr 19, 2018

gaocegege Apr 19, 2018

jlewi commented Apr 19, 2018

gaocegege left a comment

gaocegege Apr 19, 2018

jlewi Apr 19, 2018

gaocegege Apr 19, 2018

gaocegege Apr 19, 2018

jlewi Apr 19, 2018

gaocegege Apr 19, 2018

lukaszgryglicki Apr 19, 2018

jlewi Apr 19, 2018

jlewi commented Apr 19, 2018

gaocegege commented Apr 20, 2018

jlewi commented Apr 20, 2018

k8s-ci-robot commented Apr 20, 2018

		### Create the config map with the projects file

		kubectl create -n devstats configmap projects --from-file=./projects.yaml

Create a K8s app to deploy cncf/devstats for Kubeflow. #91

Create a K8s app to deploy cncf/devstats for Kubeflow. #91

Conversation

jlewi commented Apr 18, 2018 • edited Loading

k8s-ci-robot commented Apr 18, 2018

lukaszgryglicki 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gaocegege 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

jlewi commented Apr 19, 2018

gaocegege 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

jlewi commented Apr 19, 2018

gaocegege commented Apr 20, 2018

jlewi commented Apr 20, 2018

k8s-ci-robot commented Apr 20, 2018

jlewi commented Apr 18, 2018 •

edited

Loading