introduce incubator/drone #821

bacongobbler · 2017-03-20T17:40:15Z

This is a re-write of #353, focusing on chart extensibility. This has not been tested on a kubernetes cluster just yet, just PR'ing this effort for other users' enjoyment.

TODO:

rewrite README.md, documenting a basic setup guide
add ingress resource to the chart

k8s-ci-robot · 2017-03-20T17:40:23Z

Hi @bacongobbler. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with @k8s-bot ok to test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

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.

jackzampolin · 2017-03-20T17:54:40Z

@bacongobbler Might want to add an optional ingress to enable giving it a FQDN. I have a good example of this over in the chronograf chart. Makes it easy to hook into something like nginx-ingress

bacongobbler · 2017-03-20T18:07:10Z

Good point. I forgot to add that to the TODO list, but that is certainly on the radar. 👍

seanknox · 2017-03-23T00:35:26Z

incubator/drone/templates/NOTES.txt

@@ -0,0 +1,17 @@
+1. Get your Drone URL by running:
+
+{{- if contains "NodePort" .Values.serviceType }}


serviceType should be: service.type

seanknox · 2017-03-23T00:38:39Z

incubator/drone/templates/NOTES.txt

+  NOTE: It may take a few minutes for the LoadBalancer IP to be available.
+        Watch the status with: 'kubectl get svc -w {{ template "fullname" . }}'
+{{- else if contains "ClusterIP"  .Values.serviceType }}
+  export POD_NAME=$(kubectl get pods --namespace {{ .Release.Namespace }} -l "app={{ template "fullname" . }}" -o jsonpath="{.items[0].metadata.name}")


Think the label selector is supposed to be:

"app={{ template "fullname" . }}-server"

bzub · 2017-04-02T02:04:59Z

incubator/drone/values.yaml

+  # Drone server configuration. Values in here get injected as environment variables.
+  # See http://readme.drone.io/admin/installation-reference#server-options for a list of possible values.
+  env:
+    DRONE_DEBUG: "false"


I've found that I must at least add the following to get the default sqlite db driver to work:

DRONE_DATABASE_DATASOURCE: drone.sqlite

Otherwise drone-server fails to find/ping the database for some reason and crashloops. It could probably work with the default datasource (/var/lib/drone/drone.sqlite) if this chart created a volume, empty or persistent, mounted to /var/lib/drone.

This section should also include some commented out environment variables for git service setup, or at least a note that without a git service configured drone-server will fail to start.

bzub · 2017-04-02T02:56:14Z

incubator/drone/templates/service.yaml

+    port: {{ .Values.service.http.externalPort }}
+    targetPort: {{ .Values.service.http.internalPort }}
+  selector:
+    app: {{ template "fullname" . }}


selector needs to be app: {{ template "fullname" . }}-server, and the same change should probably be made for service name: and app: label.

ipedrazas · 2017-04-13T06:23:42Z

Setting up drone has a few parts:

Creating the version control webhook (GitHub, Gitlab, etc...)
Creating DNS entry for that callback (hence the comment about using an Ingress Controller)
Creating drone server using the oauth2 data

As it is, his chart doesn't work. I think it's important to have a minimal chart that works out of the box.

bacongobbler · 2017-04-13T06:53:26Z

Yes, I have been busy with other things... our company recently announced we were getting acquired by Microsoft so needless to say I've been super busy for the last bit :) hope to get back to a regular schedule soon so I can get back to this.

ipedrazas · 2017-04-13T15:20:39Z

@bacongobbler I have modified your chart to include the Ingress and the github envvars to be able to set up a simple example. @lachie83 told me to look at it and ... well, if you want my bits & bobs I can send you a PR

prydonius · 2017-05-18T16:09:47Z

@bacongobbler gentle ping - hope you're able to pick this up again!

bacongobbler · 2017-05-18T16:11:59Z

Hi! yes actually today would be a perfect day to pick this back up again. Thank you for the ping. :)

seanknox · 2017-05-18T17:28:57Z

Yay! I'm excited for this one.

bacongobbler · 2017-05-18T19:12:39Z

rebased on top of master and addressed all comments.

bacongobbler · 2017-05-18T19:21:31Z

things to note since I last worked on this:

ingress is now a feature flag
sqlite is on by default, and WILL be lost on pod restart. This also means that the drone server is not horizontally scalable as each drone server instance has its own sqlite database. This is intentional, and admins are expected to extend the chart if they need mysql/postgres (or a PR would be nice)
README was slightly vamped to demonstrate how to install this chart

seanknox · 2017-05-19T02:12:09Z

incubator/drone/Chart.yaml

+- http://readme.drone.io/
+maintainers:
+- name: Matthew Fisher
+  email: mfisher@deis.com


Can you change your name to your github handle? Recent thing we started doing.

Change your email address

seanknox · 2017-05-19T02:13:00Z

incubator/drone/README.md

+
+Alternatively, a YAML file that specifies the values for the parameters can be provided while
+installing the chart. For example,
+


A quick note about the limitations with the default DB and scaling would be helpful.

seanknox · 2017-05-19T02:15:20Z

incubator/drone/templates/deployment-server.yaml

+                key: secret
+          {{ range $key, $value := .Values.server.env }}
+          - name: {{ $key }}
+            value: {{ $value | quote }}


I like this pattern for quickly iterating over the values. Also like that the ENV vars are the defined with the caps case you'd expect in the values file itself.

seanknox · 2017-05-19T02:17:08Z

This looks great @bacongobbler. I'll try out the chart later and let you know how it goes.

seanknox · 2017-05-19T02:17:59Z

@k8s-bot ok to test

prydonius · 2017-06-01T10:38:35Z

incubator/drone/templates/deployment-agent.yaml

+        resources:
+{{ toYaml .Values.agent.resources | indent 10 }}
+        volumeMounts:
+          # Enables Docker in Docker


Just a thought, the way I've done this before (with Jenkins) is to run docker:dind in the same pod and set DOCKER_HOST in the other container to tcp://localhost:2375. IMO this is a much cleaner way of getting docker in docker on k8s.

Can you run docker-in-docker on GKE though? I thought you'd need elevated privs to run something like docker in a pod.

Yeah, the docker-in-docker container does need to be run as privileged. In my experience GKE allows this though, however other platforms may not and it will probably need to be called out in the README.

Agreed. I'll go ahead and do that. I'm on vacation atm but I'll be back next week :)

Awesome, yeah I thought you were - what are you doing on GitHub ;)?

jackzampolin · 2017-06-08T17:44:57Z

incubator/drone/templates/deployment-agent.yaml

+          - "agent"
+        env:
+          - name: DRONE_SERVER
+            value: ws://{{ template "fullname" . }}/ws/broker


Shouldn't this be ws://{{ template "fullname" . }}.{{ .Release.Namespace }}:{{ .Values.service.http.externalPort }}/ws/broker?

If the agent and server are in the same namespace (which they are), you can omit the namespace.

Duh! So used to filling in addresses this way. 🙈

seanknox · 2017-06-10T06:55:06Z

@bacongobbler server pod failed to start when I tried:

$ k get po
NAME                                                          READY     STATUS             RESTARTS   AGE
kindred-seal-nginx-ingress-controller-2980483938-jc3rl        1/1       Running            0          1h
kindred-seal-nginx-ingress-default-backend-1014810329-n4rbf   1/1       Running            0          1h
nonplussed-poodle-drone-agent-1570746251-jws7v                1/1       Running            0          6s
nonplussed-poodle-drone-server-3022101740-d11rx               0/1       CrashLoopBackOff   1          6s

$ k logs nonplussed-poodle-drone-server-3022101740-d11rx
time="2017-06-10T06:53:11Z" level=error msg="sql: unknown driver \"sqlite\" (forgotten import?)"
time="2017-06-10T06:53:11Z" level=fatal msg="database connection failed"

seanknox · 2017-06-10T19:48:04Z

Hey @bacongobbler, in case you weren't aware @ipedrazas put together a drone chart too, maybe useful for inspiration: https://github.com/kubecamp/drone-chart

viglesiasce · 2017-07-05T22:25:57Z

Ping @bacongobbler

jackzampolin · 2017-06-14T22:33:10Z

incubator/drone/templates/ingress.yaml

+      - path: /
+        backend:
+          serviceName: {{ template "fullname" . }}
+          servicePort: {{ .values.service.http.externalPort }}


Typo here (.values -> .Values)

jackzampolin · 2017-06-14T22:33:31Z

incubator/drone/values.yaml

+  enabled: false
+  # enable TLS via kube-lego
+  tls: false
+  hostname: drone.example.com


Might want to add ingress.annotations here....

bacongobbler · 2017-07-06T16:57:43Z

Couple of fixes done, as well as I changed the drone server to communicate with a docker-in-docker container running in the same pod as opposed to mounting the host socket from Kubernetes.

bacongobbler · 2017-07-06T16:59:47Z

Only problem now is to document that the administrator needs to change some fields in values.yaml in order for drone to actually work. As it stands right now doing a bare install of drone will fail because no git repo is set up by default.

><> k get po
NAME                                  READY     STATUS             RESTARTS   AGE
drone-drone-agent-1938756360-wkn0n    2/2       Running            0          7m
drone-drone-server-3036973459-wk462   0/1       CrashLoopBackOff   5          7m
><> k logs drone-drone-server-3036973459-wk462
time="2017-07-06T16:57:32Z" level=fatal msg="version control system not configured"

k8s-ci-robot · 2017-07-06T17:01:12Z

@bacongobbler: The following test failed, say /retest to rerun them all:

Test name	Commit	Details	Rerun command
Jenkins Charts e2e	440e9d64298741253a06058c68dc871fd65aa32a	link	`@k8s-bot e2e test this`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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.

bacongobbler · 2017-07-06T17:31:21Z

I've added incubator/gogs as a dependency so it'll pass CI, but it appears that gogs has some issues of its own.

><> k get po
NAME                                  READY     STATUS             RESTARTS   AGE
drone-drone-agent-3224441707-nk2x1    2/2       Running            0          3m
drone-drone-server-1297665661-xhb5c   1/1       Running            0          3m
drone-gogs-3707191466-33w69           0/1       CrashLoopBackOff   4          3m
drone-postgresql-565161641-354rh      1/1       Running            0          3m
><> k logs drone-gogs-3707191466-33w69
init:socat  | Will not try to create socat links as requested
Jul  6 17:27:46 syslogd started: BusyBox v1.25.1
Jul  6 17:27:46 sshd[24]: Server listening on :: port 22.
Jul  6 17:27:46 sshd[24]: Server listening on 0.0.0.0 port 22.

I simply don't have the time to work on this PR any more since we're no longer using drone internally. If anyone wants to take on this effort further, please let me know.

gtaylor · 2017-07-06T17:54:58Z

Whoa, I'm not sure I'd want to pull gogs in within the Drone releases we'll be creating during installation. gogs should be explicitly installed and managed in its own release. Accidentally nuking your gogs setup by removing Drone would be not great.

I assume this was meant to be an ease of use change, but I'd rather set the URL/API tokens like the other providers.

bacongobbler · 2017-07-06T18:18:58Z

It's both an ease of use change + a way to get around how CI expects every chart to work out of the box with helm install. You can't install drone without pointing at something, so I chose something local. It's still the same experience needed to switch to github/bitbucket as before, just that there's something in the first place. With github and bitbucket you need to do some external setup with developer applications. It's not something that you can just say "hey, use github out of the box, please" without doing some setup on github's end.

If you don't like the direction of this PR, please feel free to take it over!

mgoodness · 2017-09-10T21:06:05Z

Closing as stale.

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Mar 20, 2017

bacongobbler mentioned this pull request Mar 20, 2017

Drone: Creation #353

Closed

3 tasks

bacongobbler force-pushed the drone branch 2 times, most recently from 43dcc4d to c1a5208 Compare March 20, 2017 17:45

prydonius added awaiting review size/medium labels Mar 20, 2017

seanknox self-requested a review March 20, 2017 20:14

seanknox self-assigned this Mar 20, 2017

seanknox reviewed Mar 23, 2017

View reviewed changes

seanknox removed their assignment Mar 23, 2017

bzub reviewed Apr 2, 2017

View reviewed changes

prydonius added changes needed stale and removed awaiting review labels May 18, 2017

bacongobbler force-pushed the drone branch 2 times, most recently from afad83d to 621a246 Compare May 18, 2017 19:12

bacongobbler force-pushed the drone branch 2 times, most recently from 0915a3e to 440e9d6 Compare May 18, 2017 19:19

bacongobbler changed the title ~~[wip] introduce incubator/drone~~ introduce incubator/drone May 18, 2017

seanknox suggested changes May 19, 2017

View reviewed changes

prydonius reviewed Jun 1, 2017

View reviewed changes

prydonius removed the stale label Jun 1, 2017

jackzampolin reviewed Jun 8, 2017

View reviewed changes

viglesiasce added the stale label Jul 5, 2017

jackzampolin reviewed Jul 6, 2017

View reviewed changes

introduce incubator/drone

7c9d96c

bacongobbler force-pushed the drone branch from 440e9d6 to 7c9d96c Compare July 6, 2017 16:56

Matthew Fisher added 2 commits July 6, 2017 10:02

bump drone to v0.7.3

1e98bd5

add gogs as a dependency

56144b0

mgoodness closed this Sep 10, 2017

christian-roggia mentioned this pull request Oct 29, 2017

introduce incubator/drone #2614

Merged

9 tasks

		@@ -0,0 +1,17 @@
		1. Get your Drone URL by running:

		{{- if contains "NodePort" .Values.serviceType }}


		Alternatively, a YAML file that specifies the values for the parameters can be provided while
		installing the chart. For example,

introduce incubator/drone #821

introduce incubator/drone #821

Conversation

bacongobbler commented Mar 20, 2017 • edited Loading

k8s-ci-robot commented Mar 20, 2017

jackzampolin commented Mar 20, 2017

bacongobbler commented Mar 20, 2017

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

ipedrazas commented Apr 13, 2017

bacongobbler commented Apr 13, 2017

ipedrazas commented Apr 13, 2017

prydonius commented May 18, 2017

bacongobbler commented May 18, 2017

seanknox commented May 18, 2017

bacongobbler commented May 18, 2017

bacongobbler commented May 18, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

seanknox commented May 19, 2017

seanknox commented May 19, 2017

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

seanknox commented Jun 10, 2017

seanknox commented Jun 10, 2017

viglesiasce commented Jul 5, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bacongobbler commented Jul 6, 2017

bacongobbler commented Jul 6, 2017

k8s-ci-robot commented Jul 6, 2017 • edited Loading

bacongobbler commented Jul 6, 2017 • edited Loading

gtaylor commented Jul 6, 2017

bacongobbler commented Jul 6, 2017 • edited Loading

mgoodness commented Sep 10, 2017

bacongobbler commented Mar 20, 2017 •

edited

Loading

bacongobbler commented May 18, 2017 •

edited

Loading

k8s-ci-robot commented Jul 6, 2017 •

edited

Loading

bacongobbler commented Jul 6, 2017 •

edited

Loading

bacongobbler commented Jul 6, 2017 •

edited

Loading