Add migration command #870

shawn-hurley · 2018-04-02T16:27:09Z

Describe what this PR does and why we need it:

We need a way to migrate from ETCD to CRD for openshift ansible

Changes proposed in this pull request

create a new command and add to RPM and Images
connects to etcd and crd dao's and will convert etcd to crd

example job.yaml:

---
apiVersion: batch/v1
kind: Job
metadata:
  name: asb-etcd-migration
spec:
  parallelism: 1
  completions: 1
  template:
    metadata:
      name: asb-etcd-migration
    spec:
      containers:
        - name: asb
          image: 'docker.io/shurley/origin-ansible-service-broker:migration'
          imagePullPolicy: Always
          # TODO: Need to update to proper entrypoint once it exists
          command:
            - '/usr/bin/migration'
          args:
            - '-host=asb-etcd.ansible-service-broker.svc'
            - '-ca-file=/var/run/secrets/kubernetes.io/serviceaccount/service-ca.crt'
            - '-client-cert=/var/run/asb-etcd-auth/client.crt'
            - '-client-key=/var/run/asb-etcd-auth/client.key'
            - '-namespace=ansible-service-broker'
          volumeMounts:
          - name: config-volume
            mountPath: /etc/ansible-service-broker
          - name: asb-tls
            mountPath: /etc/tls/private
          - name: asb-etcd-auth
            mountPath: /var/run/asb-etcd-auth
          env:
            - name: BROKER_CONFIG
              value: /etc/ansible-service-broker/config.yaml
            - name: HTTP_PROXY
              value: ""
            - name: HTTPS_PROXY
              value: ""
      volumes:
        - name: config-volume
          configMap:
            name: broker-config
            items:
              - key: broker-config
                path: config.yaml
        - name: asb-tls
          secret:
            secretName: asb-tls
        - name: asb-etcd-auth
          secret:
            defaultMode: 420
            secretName: broker-etcd-auth-secret
      restartPolicy: Never
      serviceAccount: asb
      serviceAccountName: asb

This will be used to migrate from etcd to crd

rthallisey · 2018-04-02T17:20:09Z

cmd/migration/main.go

+	}
+	crdDao, err = crd.NewDao(options.MigrationNamespace)
+	if err != nil {
+		panic(fmt.Sprintf("Unable to connect to crd - %v", err))


NIT: I don't think this connects to the crd resource. It only creates the client. Maybe a different error message should be here.

Waiting on more reviews before updating this one

@shawn-hurley might also be worth a precheck on the bundle resource. Maybe something like this:

_, err := resources.Bundle.Bundles(c.config.Namespace).List(metav1.ListOptions{}) if err != nil { .... }

If the bundles already exist it will error and the migration will fail, I think it would be better to attempt to migrate IMO.

jmrodri

Looking at the code portion now. There are changes to the makefile and spec file needed.

jmrodri · 2018-04-02T17:41:09Z

Makefile

@@ -19,6 +19,7 @@ vendor: ## Install or update project dependencies

 broker: $(SOURCES) ## Build the broker
 	go build -i -ldflags="-s -w" ./cmd/broker
+	go build -i -ldflags="-s -w" ./cmd/migration


this doesn't feel right. The broker Makefile target is the ACTUAL broker binary. This target says build the broker binary if any of the $(SOURCES) have changed.

I would expect the following:

migration: $(SOURCES) ## Build the migration tool go build -i -ldflags="-s -w" ./cmd/migration

jmrodri · 2018-04-02T17:41:31Z

Makefile

@@ -69,6 +70,7 @@ prep-local: ## Prepares the local dev environment

 build-image: ## Build a docker image with the broker binary
 	env GOOS=linux go build -i -ldflags="-s -s" -o ${BUILD_DIR}/broker ./cmd/broker
+	env GOOS=linux go build -i -ldflags="-s -s" -o ${BUILD_DIR}/migration ./cmd/migration


jmrodri · 2018-04-02T17:43:38Z

ansible-service-broker.spec

@@ -178,6 +179,7 @@ rm -rf src
 %install
 install -d -p %{buildroot}%{_bindir}
 install -p -m 755 broker %{buildroot}%{_bindir}/asbd
+install -p -m 755 migration %{buildroot}%{_bindir}/migration


you need a line under the %files section for the migration

%{_bindir}/migration `` Similar to `asbd` https://github.com/shawn-hurley/ansible-service-broker/blob/11b100561dc09b374a2a76aef5e99a0364e77b8d/ansible-service-broker.spec#L283

jmrodri · 2018-04-02T17:44:07Z

build/Dockerfile-canary

@@ -43,6 +43,7 @@ WORKDIR /go/src/github.com/openshift/ansible-service-broker

 RUN go build -i -ldflags="-s -w" ./cmd/broker
 RUN mv broker /usr/bin/asbd
+RUN mv migration /usr/bin/migration


jmrodri · 2018-04-02T17:44:14Z

build/Dockerfile-localdev

@@ -23,6 +23,7 @@ RUN mkdir /var/log/ansible-service-broker \

 COPY entrypoint.sh /usr/bin/
 COPY broker /usr/bin/asbd
+COPY migration /usr/bin/migration


jmrodri · 2018-04-02T17:46:33Z

cmd/migration/main.go

+
+import (
+	"encoding/json"
+	"flag"


stock flag is probably okay for this. But it is weird to have a different one than we are using for the broker:

"github.com/jessevdk/go-flags"

Not a blocker just pointing it out.

jmrodri · 2018-04-02T18:08:30Z

The rest looks okay.

jmrodri · 2018-04-02T19:41:02Z

Add migration to the clean target @rm -f broker migration. Once that is done, merge it yourself.

shawn-hurley and others added 2 commits March 30, 2018 15:27

first pass at migration script

ae31d28

Adding migration script to the image.

11b1005

This will be used to migrate from etcd to crd

openshift-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Apr 2, 2018

fabianvf mentioned this pull request Apr 2, 2018

Add logic for new default CRD backend for ASB openshift/openshift-ansible#7709

Merged

5 tasks

rthallisey reviewed Apr 2, 2018

View reviewed changes

jmrodri suggested changes Apr 2, 2018

View reviewed changes

Addressing pr review comments

145f6e9

rthallisey approved these changes Apr 2, 2018

View reviewed changes

jmrodri approved these changes Apr 2, 2018

View reviewed changes

jmrodri added the build label Apr 2, 2018

adding migration to the clean target

579f754

eriknelson approved these changes Apr 3, 2018

View reviewed changes

shawn-hurley merged commit adfa7f4 into openshift:master Apr 3, 2018

Add migration command #870

Add migration command #870

shawn-hurley commented Apr 2, 2018 •

edited

rthallisey Apr 2, 2018

shawn-hurley Apr 2, 2018

rthallisey Apr 2, 2018

shawn-hurley Apr 2, 2018

jmrodri left a comment

jmrodri Apr 2, 2018

jmrodri Apr 2, 2018

jmrodri Apr 2, 2018

jmrodri Apr 2, 2018

jmrodri Apr 2, 2018

jmrodri Apr 2, 2018

jmrodri commented Apr 2, 2018

jmrodri commented Apr 2, 2018

Add migration command #870

Add migration command #870

Conversation

shawn-hurley commented Apr 2, 2018 • edited

Describe what this PR does and why we need it:

Changes proposed in this pull request

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

jmrodri 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

jmrodri commented Apr 2, 2018

jmrodri commented Apr 2, 2018

shawn-hurley commented Apr 2, 2018 •

edited