Setup release process by smacker · Pull Request #30 · src-d/code-annotation

smacker · 2018-01-24T11:42:19Z

Fixes: #8

TODO (need infra):

Move from quay.io to docker hub
Test on minikube
Test helm chart on cluster with ingress

bzz · 2018-01-26T10:54:42Z

AFAIK current setup is copied from https://github.com/src-d/landing/

\cc @rporres for next steps

dpordomingo

I only performed a high-level review, and saw that the changes made are only:

removing not needed services: landingApi and slackin,
renaming landing by code-annotation
what makes sense for code-annotation necessities.

I only spotted some inconsistencies regarding hostnames for staging, so in general LGTM

dpordomingo · 2018-01-30T12:25:47Z

+    tiller_ns: kube-system
+    wait: true
+    when:
+      event: [tag]


missing EOL

dpordomingo · 2018-01-30T12:38:53Z

+  build_stg:
+    image: golang:1.8-alpine3.6
+    environment:
+      - REACT_APP_SERVER_URL="//code-annotation.srcd.run"


expected url would be //code-annotation-staging.srcd.run
same pattern than for: talks-staging.srcd.run, blog-staging.srcd.run, and landing-staging.srcd.run
https://github.com/src-d/guide/blob/master/engineering/continuous-delivery.md

dpordomingo · 2018-01-30T12:45:07Z

+    release: code-annotation
+    prefix: STG
+    secrets: [ STG_API_SERVER, STG_KUBERNETES_TOKEN ]
+    values: ingress.globalStaticIpName=code-annotation-staging,ingress.hostname=code-annotation.srcd.run,image.tag=commit-${DRONE_COMMIT_SHA:0:7}


expected ingress.hostname would be expected url would be code-annotation-staging.srcd.run

sure if I change it, I'll change it everywhere

bzz · 2018-01-30T13:48:14Z

+    group: docker
+    image: plugins/docker
+    registry: quay.io
+    repo: quay.io/srcd/code-annotation


Change to DockerHub

bzz · 2018-01-30T13:48:28Z

+
+  # deployment to production environment
+
+  build_prod:


remove production

rporres

Looks good. A couple of details to be addressed.

rporres · 2018-01-31T11:57:49Z

+ingress:
+  annotations:
+    kubernetes.io/ingress.class: gce
+    kubernetes.io/tls-acme: "true"


There was no way for you to know this, but we're moving away from kube-lego to handle Let's Encrypt certificates as it has given us some grief in the past and it's no longer under development. We're using kube-cert-manager. See https://github.com/src-d/docs/tree/master/helm-charts/docs

rporres · 2018-01-31T12:02:04Z

+  pullPolicy: IfNotPresent
+  registry: docker.io/srcd
+  codeAnnotation:
+    name: code-annotation


Are you planning to have multiple images for this in the future? If not, this is not necessary. It was done that way in landing as deployment is composed of multiple containers. The standard way of declaring this is using image.repository and image.tag, e.g. https://github.com/src-d/docs/blob/master/helm-charts/docs/values.yaml#L6-L7 and https://github.com/src-d/docs/blob/master/helm-charts/docs/templates/deployment.yaml#L20

smacker · 2018-01-31T13:46:41Z

Addressed. PR waits for published docker image and cluster with ingress.

smacker · 2018-01-31T16:49:44Z

@rporres I just updated PR. There are secrets are required now.

And you can take image here:
https://hub.docker.com/r/smacker/code-annotation/tags/

rporres · 2018-02-01T10:45:12Z

We have a problem with those secrets due to ipedrazas/drone-helm#52. The only workaround is for the chart to be able to accept a secret already deployed in the cluster.

If you want to keep your chart flexible enough (which I think it is good) I would modify the chart to:

only create the secret if .Values.secrets.github_client and its friends are present (you can add a condition surrounding the whole secret definition in templates/secrets.yaml)
define a variable containing the name of the secret at the beginning of templates/deployment.yaml based on a condition that would test if an existing secret is given or secret values are provided. See this as an example on how to define variables in a chart

The above solution would only work if the existing secret structure is the same as the secret already present in the cluster, so that would also need be documented.

I'm sorry that you've hit that nasty drone-helm bug

bzz · 2018-02-07T14:37:37Z

It seems that requested changes were done to work it round, so from conversation IRL \w @smacker we are just waiting for @rporres have some spare cycles to help us finishing testing it - the last item from PR todo in description.

rporres · 2018-02-07T15:17:07Z

Now that I'm back to work I will take a look into this

rporres · 2018-02-07T16:08:16Z

ingress metadata is missing the line

stable.k8s.psg.io/kcm.class: {{ .Values.ingress.kcmClass }}

rporres · 2018-02-07T16:15:47Z

https://code-annotation-staging.srcd.run/ and https://code-annotation-staging.srcd.run/ work with the ingress fix above. Since we cannot force redirections in GCE ingress, it would be nice that the application did it

rporres · 2018-02-07T16:16:52Z

I still have to configure drone, so please do not merge it

smacker · 2018-02-07T16:21:24Z

kcm.class added. I also rebased and squshed commits.

rporres

Drone set up. Feel free to merge it.

bzz

🎉 nice!
Looks great to me.

bzz mentioned this pull request Jan 26, 2018

Design polishing #19

Closed

dpordomingo reviewed Jan 30, 2018

View reviewed changes

bzz reviewed Jan 30, 2018

View reviewed changes

Comment thread .drone.yml Outdated

# deployment to production environment

build_prod:

Copy link
Copy Markdown

Contributor

bzz Jan 30, 2018

Choose a reason for hiding this comment

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

remove production

smacker force-pushed the drone branch from 4a0f391 to e762375 Compare January 30, 2018 16:50

rporres suggested changes Jan 31, 2018

View reviewed changes

smacker force-pushed the drone branch from 75cacea to 14831ba Compare January 31, 2018 17:28

dpordomingo approved these changes Feb 1, 2018

View reviewed changes

smacker force-pushed the drone branch from 127474f to dd8911e Compare February 7, 2018 16:19

smacker changed the title ~~[WIP] Setup release process~~ Setup release process Feb 7, 2018

Setup release process

c76bf02

smacker force-pushed the drone branch from dd8911e to c76bf02 Compare February 7, 2018 16:22

rporres approved these changes Feb 7, 2018

View reviewed changes

dpordomingo mentioned this pull request Feb 8, 2018

Prepare first release of code annotation #70

Closed

16 tasks

bzz approved these changes Feb 8, 2018

View reviewed changes

bzz merged commit ade3f2c into src-d:master Feb 9, 2018

Conversation

smacker commented Jan 24, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bzz commented Jan 26, 2018

Uh oh!

dpordomingo left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rporres left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

smacker commented Jan 31, 2018

Uh oh!

smacker commented Jan 31, 2018

Uh oh!

rporres commented Feb 1, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bzz commented Feb 7, 2018

Uh oh!

rporres commented Feb 7, 2018

Uh oh!

rporres commented Feb 7, 2018

Uh oh!

rporres commented Feb 7, 2018

Uh oh!

rporres commented Feb 7, 2018

Uh oh!

smacker commented Feb 7, 2018

Uh oh!

rporres left a comment

Choose a reason for hiding this comment

Uh oh!

bzz left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

smacker commented Jan 24, 2018 •

edited

Loading

dpordomingo left a comment •

edited

Loading

rporres commented Feb 1, 2018 •

edited

Loading