Add ability in backend to upgrade GitLab Runnner application
What does this MR do?
Adds BE code to upgrade the GitLab runner application
Something in a later MR (periodic worker?, migration?) will then have to call Clusters::Applications::UpdateService
.
What are the relevant issue numbers?
https://gitlab.com/gitlab-org/gitlab-ce/issues/49384
Does this MR meet the acceptance criteria?
-
Changelog entry added, if necessary -
Documentation created/updated via this MR -
Tests added for this feature/bug -
Tested in all supported browsers -
Conforms to the code review guidelines -
Conforms to the merge request performance guidelines -
Conforms to the style guides -
Conforms to the database guides -
Link to e2e tests MR added if this MR has Requires e2e tests label. See the Test Planning Process. -
Security reports checked/validated by reviewer
Merge request reports
Activity
changed milestone to %11.8
added Category:Kubernetes Management Deliverable UX backend customer depth devopsconfigure [DEPRECATED] direction missed-deliverable missed:11.7 + 1 deleted label
logs from
upgrade-runner
pod :Client: &version.Version{SemVer:"v2.11.0", GitCommit:"2e55dbe1fdb5fdb96b75ff144a339489417b146b", GitTreeState:"clean"} Error: cannot connect to Tiller + sleep 1s Retrying (30)... + echo 'Retrying (30)...' + helm repo add runner https://charts.gitlab.io "runner" has been added to your repositories + helm repo update Hang tight while we grab the latest from your chart repositories... ...Skip local chart repository ...Successfully got an update from the "stable" chart repository ...Successfully got an update from the "runner" chart repository Update Complete. ⎈ Happy Helming!⎈ + helm upgrade runner runner/gitlab-runner --version 0.1.44 --tls --tls-ca-cert /data/helm/runner/config/ca.pem --tls-cert /data/helm/runner/config/cert.pem --tls-key /data/helm/runner/config/key.pem --reset-values --install --namespace gitlab-managed-apps -f /data/helm/runner/config/values.yaml Release "runner" has been upgraded. Happy Helming! E0117 02:55:24.411673 1 portforward.go:303] error copying from remote stream to local connection: readfrom tcp4 127.0.0.1:45371->127.0.0.1:45820: write tcp4 127.0.0.1:45371->127.0.0.1:45820: write: broken pipe LAST DEPLOYED: Thu Jan 17 02:55:24 2019 NAMESPACE: gitlab-managed-apps STATUS: DEPLOYED RESOURCES: ==> v1/Secret NAME AGE runner-gitlab-runner 25m ==> v1/ConfigMap runner-gitlab-runner 25m ==> v1beta1/Deployment runner-gitlab-runner 25m NOTES: Your GitLab Runner should now be registered against the GitLab instance reachable at: "https://24046.qa-tunnel.gitlab.info/"
However, I am not getting a new gitlab-runner pod after upgrade :(
tkgl:gitlab tkuah$ kc get deployment -n gitlab-managed-apps NAME DESIRED CURRENT UP-TO-DATE AVAILABLE AGE runner-gitlab-runner 1 0 0 0 26m tiller-deploy 1 1 1 1 33m
Error below:
$ kubectl get deployment runner-gitlab-runner -n gitlab-managed-apps -o yaml # snip status: conditions: - lastTransitionTime: 2019-01-17T02:30:01Z lastUpdateTime: 2019-01-17T02:30:01Z message: Deployment has minimum availability. reason: MinimumReplicasAvailable status: "True" type: Available - lastTransitionTime: 2019-01-17T02:41:04Z lastUpdateTime: 2019-01-17T02:41:04Z message: 'pods "runner-gitlab-runner-676ddb4c7-" is forbidden: error looking up service account gitlab-managed-apps/runner-gitlab-runner: serviceaccount "runner-gitlab-runner" not found' reason: FailedCreate status: "True" type: ReplicaFailure - lastTransitionTime: 2019-01-17T02:51:05Z lastUpdateTime: 2019-01-17T02:51:05Z message: ReplicaSet "runner-gitlab-runner-676ddb4c7" has timed out progressing. reason: ProgressDeadlineExceeded status: "False" type: Progressing observedGeneration: 2 unavailableReplicas: 1
Edited by Thong KuahSpot the difference ! Looks like
Gitlab::Kubernetes::Helm::UpgradeCommand
was missingrbac.create
flags[35] pry(main)> puts runner.upgrade_command.generate_script set -xeo pipefail helm init --upgrade for i in $(seq 1 30); do helm version && break; sleep 1s; echo "Retrying ($i)..."; done helm repo add runner https://charts.gitlab.io helm repo update helm upgrade runner runner/gitlab-runner --version 0.1.44 --tls --tls-ca-cert /data/helm/runner/config/ca.pem --tls-cert /data/helm/runner/config/cert.pem --tls-key /data/helm/runner/config/key.pem --reset-values --install --namespace gitlab-managed-apps -f /data/helm/runner/config/values.yaml
=> nil [36] pry(main)> puts runner.install_command.generate_script set -xeo pipefail helm init --upgrade for i in $(seq 1 30); do helm version && break; sleep 1s; echo "Retrying ($i)..."; done helm repo add runner https://charts.gitlab.io helm repo update helm install runner/gitlab-runner --name runner --tls --tls-ca-cert /data/helm/runner/config/ca.pem --tls-cert /data/helm/runner/config/cert.pem --tls-key /data/helm/runner/config/key.pem --version 0.1.44 --set rbac.create\=true,rbac.enabled\=true --namespace gitlab-managed-apps -f /data/helm/runner/config/values.yaml
43 " --install" \ 44 " --namespace #{::Gitlab::Kubernetes::Helm::NAMESPACE}" \ 45 " -f /data/helm/#{name}/config/values.yaml" 41 def repository_update_command 42 'helm repo update' if repository 43 end 44 45 def upgrade_command 46 command = ["helm", "upgrade", name, chart] + helm_upgrade_flags 47 48 command.shelljoin 49 end 46 50 47 "helm upgrade #{name} #{chart}#{upgrade_flags}" 51 def helm_upgrade_flags 52 reset_values_flag = ['--reset-values'] Do we need these extra variables? Should we consider just returning the literal array plus all the method calls.
Edited by Dylan Griffith
55 return unless files.key?(:'ca.pem') 81 return [] unless files.key?(:'ca.pem') 56 82 57 " --tls" \ 58 " --tls-ca-cert #{files_dir}/ca.pem" \ 59 " --tls-cert #{files_dir}/cert.pem" \ 60 " --tls-key #{files_dir}/key.pem" 83 [ 84 '--tls', 85 '--tls-ca-cert', "#{files_dir}/ca.pem", 86 '--tls-cert', "#{files_dir}/cert.pem", 87 '--tls-key', "#{files_dir}/key.pem" 88 ] 61 89 end 62 90 end 63 91 end The following discussion from !23924 (merged) should be addressed:
-
@ayufan started a discussion:
Why this is
""
instead of{}
?
-
mentioned in merge request !23924 (merged)
marked the checklist item Changelog entry added, if necessary as completed
added 160 commits
Toggle commit listadded 1 commit
- 2ef6efcc - Port upgrade check worker and service from EE
3 Warnings ⚠ This merge request is quite big (more than 768 lines changed), please consider splitting it into multiple merge requests. ⚠ This merge request is missing the database label. ⚠ This merge request adds files that do not enforce frozen string literal. See https://gitlab.com/gitlab-org/gitlab-ce/issues/47424 for more information. 1 Message 📖 This merge request adds or changes files that require a review from the Database team. Database Review
The following files require a review from the Database team:
db/migrate/20190123004235_add_last_update_started_at_to_applications_prometheus_ce.rb
db/schema.rb
To make sure these changes are reviewed, take the following steps:
- Edit your merge request, and add
gl-database
to the list of Group approvers. - Mention
@gl-database
in a separate comment, and explain what needs to be reviewed by the team. Please don't mention the team until your changes are ready for review.
Enable Frozen String Literal
The following files should have
# frozen_string_literal: true
on the first line:spec/services/clusters/applications/schedule_update_service_spec.rb
spec/workers/cluster_update_app_worker_spec.rb
Generated by
🚫 DangerEdited by 🤖 GitLab Bot 🤖3 module Clusters 4 module Applications 5 class CheckUpgradeProgressService < BaseHelmService 6 def execute 7 return unless app.updating? 8 9 case phase 10 when ::Gitlab::Kubernetes::Pod::SUCCEEDED 11 on_success 12 when ::Gitlab::Kubernetes::Pod::FAILED 13 on_failed 14 else 15 check_timeout 16 end 17 rescue ::Kubeclient::HttpError => e 18 app.make_update_errored!("Kubernetes error: #{e.message}") unless app.update_errored? 15 check_timeout 16 end 17 rescue ::Kubeclient::HttpError => e 18 app.make_update_errored!("Kubernetes error: #{e.message}") unless app.update_errored? 19 end 20 21 private 22 23 def on_success 24 app.make_updated! 25 ensure 26 remove_pod 27 end 28 29 def on_failed 30 app.make_update_errored!(errors || 'Update silently failed') added 97 commits
-
eba726c7...5cf99404 - 91 commits from branch
master
- 4e8189c8 - Port upgrade workers and services from EE
- 8650ed14 - Convert flags to use shelljoin
- 36177cbd - Teach runner app about #upgrade_command
- 087de562 - Add missing rbac.create flags
- 79854159 - Reduce long lines in spec
- 617015e0 - Add service to upgrade cluster applications
Toggle commit list-
eba726c7...5cf99404 - 91 commits from branch
Splitting out EE port to https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/24589
This is a dead-end, superseded by https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/24589
added groupconfigure [DEPRECATED] label
added groupenvironments label and removed groupconfigure [DEPRECATED] label