more pr review

Shopify · Jan 18, 2019 · e4dbe90 · e4dbe90
1 parent 796021d
commit e4dbe90
Show file tree

Hide file tree

Showing 5 changed files with 8 additions and 34 deletions.
diff --git a/lib/kubernetes-deploy/kubernetes_resource/custom_resource.rb b/lib/kubernetes-deploy/kubernetes_resource/custom_resource.rb
@@ -64,8 +64,11 @@ def validate_definition(kubectl)
 
       @crd.validate_rollout_conditions
     rescue RolloutConditionsError => e
-      @validation_errors << "Annotation #{CustomResourceDefinition::ROLLOUT_CONDITIONS_ANNOTATION} " \
-      "on #{@crd.name} is invalid: #{e}"
+      @validation_errors << "The CRD that specifies this resource is using invalid rollout conditions. " \
+      "Kubernetes-deploy will not be able to continue until those rollout conditions are fixed.\n" \
+      "Rollout conditions can be found on the CRD that defines this resource (#{@crd.name}), " \
+      "Under the annotation #{CustomResourceDefinition::ROLLOUT_CONDITIONS_ANNOTATION}.\n" \
+      "Validation failed with: #{e}"
     end
 
     private

diff --git a/lib/kubernetes-deploy/kubernetes_resource/custom_resource_definition.rb b/lib/kubernetes-deploy/kubernetes_resource/custom_resource_definition.rb
@@ -79,6 +79,7 @@ def validate_rollout_conditions
         conditions = RolloutConditions.from_annotation(rollout_conditions_annotation)
         conditions.validate!
       end
+      # ensure
       @rollout_conditions_validated = true
     end
 

diff --git a/test/fixtures/crd/with_custom_conditions.yml b/test/fixtures/crd/with_custom_conditions.yml
@@ -3,6 +3,7 @@ kind: CustomResourceDefinition
 metadata:
   name: customizeds.stable.example.io
   annotations:
+    kubernetes-deploy.shopify.io/instance-timeout: '5S'
     kubernetes-deploy.shopify.io/instance-rollout-conditions: '{
       "success_conditions": [
         {

diff --git a/test/integration-serial/serial_deploy_test.rb b/test/integration-serial/serial_deploy_test.rb
@@ -3,7 +3,7 @@
 
 class SerialDeployTest < KubernetesDeploy::IntegrationTest
   include StatsDHelper
-  This cannot be run in parallel because it either stubs a constant or operates in a non-exclusive namespace
+  # This cannot be run in parallel because it either stubs a constant or operates in a non-exclusive namespace
   def test_deploying_to_protected_namespace_with_override_does_not_prune
     KubernetesDeploy::DeployTask.stub_const(:PROTECTED_NAMESPACES, [@namespace]) do
       assert_deploy_success(deploy_fixtures("hello-cloud", subset: ['configmap-data.yml', 'disruption-budgets.yml'],
@@ -326,36 +326,6 @@ def test_cr_success_with_arbitrary_rollout_conditions
     wait_for_all_crd_deletion
   end
 
-  def test_cr_success_with_only_success_rollout_conditions
-    crd_result = deploy_fixtures("crd", subset: ["with_custom_conditions.yml"]) do |resource|
-      crd = resource["with_custom_conditions.yml"]["CustomResourceDefinition"].first
-      crd["metadata"]["annotations"][KubernetesDeploy::CustomResourceDefinition::ROLLOUT_CONDITIONS_ANNOTATION] = {
-        success_conditions: [{ path: "$.status.test_field", value: "success_value" }],
-      }.to_json
-    end
-    assert_deploy_success(crd_result)
-
-    success_conditions = {
-      "spec" => {},
-      "status" => {
-        "observedGeneration" => 1,
-        "test_field" => "success_value",
-        "condition" => "success_value",
-      },
-    }
-
-    result = deploy_fixtures("crd", subset: ["with_custom_conditions_cr.yml"]) do |resource|
-      cr = resource["with_custom_conditions_cr.yml"]["Customized"].first
-      cr.merge!(success_conditions)
-    end
-    assert_deploy_success(result)
-    assert_logs_match_all([
-      %r{Successfully deployed in .*: Customized\/with-customized-params},
-    ])
-  ensure
-    wait_for_all_crd_deletion
-  end
-
   def test_cr_failure_with_arbitrary_rollout_conditions
     assert_deploy_success(deploy_fixtures("crd", subset: ["with_custom_conditions.yml"]))
     cr = load_fixtures("crd", ["with_custom_conditions_cr.yml"])

diff --git a/test/unit/kubernetes-deploy/kubernetes_resource/custom_resource_definition_test.rb b/test/unit/kubernetes-deploy/kubernetes_resource/custom_resource_definition_test.rb
@@ -2,7 +2,6 @@
 require 'test_helper'
 
 class CustomResourceDefinitionTest < KubernetesDeploy::TestCase
-
   def test_rollout_conditions_nil_when_none_present
     crd = build_crd(crd_spec)
     refute(crd.rollout_conditions)