Merge branch 'retry-review-deploy-twice' into 'master'

Retry the review-deploy job twice and make the Review App cleanup more efficient

See merge request gitlab-org/gitlab-ce!27489

.gitlab/ci/review.gitlab-ci.yml

@@ -32,7 +32,6 @@
   environment: &review-environment
     name: review/${CI_COMMIT_REF_NAME}
     url: https://gitlab-${CI_ENVIRONMENT_SLUG}.${REVIEW_APPS_DOMAIN}
-  before_script: []
 
 .review-docker: &review-docker
   <<: *review-base
@@ -48,6 +47,7 @@
     DOCKER_HOST: tcp://docker:2375
     LATEST_QA_IMAGE: "gitlab/${CI_PROJECT_NAME}-qa:nightly"
     QA_IMAGE: "${CI_REGISTRY}/${CI_PROJECT_PATH}/gitlab/${CI_PROJECT_NAME}-qa:${CI_COMMIT_REF_SLUG}"
+  before_script: []
 
 build-qa-image:
   <<: *review-docker
@@ -85,6 +85,7 @@ schedule:review-build-cng:
 
 .review-deploy-base: &review-deploy-base
   <<: *review-base
+  retry: 2
   allow_failure: true
   variables:
     HOST_SUFFIX: "${CI_ENVIRONMENT_SLUG}"

lib/quality/kubernetes_client.rb

@@ -18,6 +18,8 @@ module Quality
         'delete',
         'ingress,svc,pdb,hpa,deploy,statefulset,job,pod,secret,configmap,pvc,secret,clusterrole,clusterrolebinding,role,rolebinding,sa',
         '--now',
+        '--ignore-not-found',
+        '--include-uninitialized',
         %(-l release="#{release_name}")
       ]
 

scripts/review_apps/review-apps.sh

@@ -68,7 +68,7 @@ function delete() {
 
   echoinfo "Deleting release '$name'..." true
 
-  helm delete --purge "$name" || true
+  helm delete --purge "$name"
 }
 
 function cleanup() {
@@ -81,8 +81,8 @@ function cleanup() {
 
   kubectl -n "$KUBE_NAMESPACE" delete \
     ingress,svc,pdb,hpa,deploy,statefulset,job,pod,secret,configmap,pvc,secret,clusterrole,clusterrolebinding,role,rolebinding,sa \
-    -l release="$CI_ENVIRONMENT_SLUG" \
-    || true
+    --now --ignore-not-found --include-uninitialized \
+    -l release="$CI_ENVIRONMENT_SLUG"
 }
 
 function get_pod() {

spec/lib/quality/kubernetes_client_spec.rb

@@ -13,7 +13,7 @@ RSpec.describe Quality::KubernetesClient do
       expect(Gitlab::Popen).to receive(:popen_with_detail)
         .with([%(kubectl --namespace "#{namespace}" delete ) \
           'ingress,svc,pdb,hpa,deploy,statefulset,job,pod,secret,configmap,pvc,secret,clusterrole,clusterrolebinding,role,rolebinding,sa ' \
-          "--now -l release=\"#{release_name}\""])
+          "--now --ignore-not-found --include-uninitialized -l release=\"#{release_name}\""])
         .and_return(Gitlab::Popen::Result.new([], '', '', double(success?: false)))
 
       expect { subject.cleanup(release_name: release_name) }.to raise_error(described_class::CommandFailedError)
@@ -23,7 +23,7 @@ RSpec.describe Quality::KubernetesClient do
       expect(Gitlab::Popen).to receive(:popen_with_detail)
         .with([%(kubectl --namespace "#{namespace}" delete ) \
           'ingress,svc,pdb,hpa,deploy,statefulset,job,pod,secret,configmap,pvc,secret,clusterrole,clusterrolebinding,role,rolebinding,sa ' \
-          "--now -l release=\"#{release_name}\""])
+          "--now --ignore-not-found --include-uninitialized -l release=\"#{release_name}\""])
         .and_return(Gitlab::Popen::Result.new([], '', '', double(success?: true)))
 
       # We're not verifying the output here, just silencing it