Predeploy more resources

1.0-Upgrade.md

@@ -7,7 +7,7 @@
 * There are breaking changes in the public API (such as the renaming of the `KubernetesDeploy` namespace to `Krane`, and the change in default values for different arguments of the public interface).
 * StatsD metrics will now be generated with the `krane` prefix.
 * `krane deploy` now considers all namespaced resources eligible for pruning, including
-custom resources. See [blacklist](https://github.com/Shopify/krane/blob/main/lib/krane/cluster_resource_discovery.rb#L20) for exceptions.
+custom resources. See [blacklist](https://github.com/Shopify/krane/blob/main/lib/krane/cluster_resource_discovery.rb#L22) for exceptions.
 * `kubernetes-deploy` (now `krane deploy`) / `DeployTask` can no longer deploy global (non-namespaced) resources. A new command called `krane global-deploy` and a related class called `GlobalDeployTask` were added to replace that feature.
 * `krane deploy` will not render erb templates. Use `krane render | krane deploy --stdin` to reproduce this functionality.
 * If you attempt to install two gems that have conflicting executables, `gem install` will warn you but the most recently installed one will win.

CONTRIBUTING.md

@@ -91,12 +91,13 @@ This gem uses subclasses of `KubernetesResource` to implement custom success/fai
 1. Create a file for your type in `lib/krane/kubernetes_resource/`
 2. Create a new class that inherits from `KubernetesResource`. Minimally, it should implement the following methods:
     * `sync` -- Gather the data you'll need to determine `deploy_succeeded?` and `deploy_failed?`. The superclass's implementation fetches the corresponding resource, parses it and stores it in `@instance_data`. You can define your own implementation if you need something else.
+    * `predeployed?` -- Whether the resource should be [predeployed](README.md#phase-3-predeploying-priority-resources) by default.
     * `deploy_succeeded?`
     * `deploy_failed?`
 3. Adjust the `TIMEOUT` constant to an appropriate value for this type.
 4. Add the new class to list of resources in
    [`deploy_task.rb`](https://github.com/Shopify/krane/blob/main/lib/krane/deploy_task.rb#L8)
-5. Add the new resource to the [prune whitelist](https://github.com/Shopify/krane/blob/main/lib/krane/deploy_task.rb#L81)
+5. Add the new resource to the [prune whitelist](https://github.com/Shopify/krane/blob/main/lib/krane/deploy_task.rb#L93)
 6. Add a basic example of the type to the hello-cloud [fixture set](https://github.com/Shopify/krane/tree/main/test/fixtures/hello-cloud) and appropriate assertions to `#assert_all_up` in [`hello_cloud.rb`](https://github.com/Shopify/krane/blob/main/test/helpers/fixture_sets/hello_cloud.rb). This will get you coverage in several existing tests, such as `test_full_hello_cloud_set_deploy_succeeds`.
 7. Add tests for any edge cases you foresee.
 

README.md

@@ -166,11 +166,11 @@ before the deployment is considered successful.
     - Percent (e.g. 90%): The deploy is successful when the number of new pods that are ready is equal to `spec.replicas` * Percent.
   - _Compatibility_: StatefulSet
     - `full`: The deployment is successful when all pods are ready.
-- `krane.shopify.io/predeployed`: Causes a Custom Resource to be deployed in the pre-deploy phase.
-  - _Compatibility_: Custom Resource Definition
-  - _Default_: `true`
-  - `true`: The custom resource will be deployed in the pre-deploy phase.
-  - All other values: The custom resource will be deployed in the main deployment phase.
+- `krane.shopify.io/predeployed`: Causes a Custom Resource or [other resources](https://github.com/powerhome/krane/tree/main/lib/krane/kubernetes_resource) to be deployed in the pre-deploy phase.
+  - _Compatibility_: CronJob, CustomResource, CustomResourceDefinition, DaemonSet, Deployment, HorizontalPodAutoscaler, Ingress, Job, Pod, PodDisruptionBudget, PodSetBase, PodTemplate, ReplicaSet, Service, StatefulSet
+  - _Default_: `false`
+  - `true`: The custom resource or other resource will be deployed in the pre-deploy phase.
+  - All other values: The custom resource or other resource will be deployed in the main deployment phase.
 - `krane.shopify.io/deploy-method-override`: Cause a resource to be deployed by the specified `kubectl` command, instead of the default `apply`.
   - _Compatibility_: Cannot be used for `PodDisruptionBudget`, since it always uses `create/replace-force`
   - _Accepted values_: `create`, `replace`, and `replace-force`

lib/krane/deploy_task.rb

@@ -70,7 +70,19 @@ def predeploy_sequence
       ).map { |r| [r, default_group] }
 
       after_crs = %w(
+        Deployment
+        Service
+        Ingress
         Pod
+        Job
+        CronJob
+        DaemonSet
+        HorizontalPodAutoscaler
+        PodDisruptionBudget
+        PodSetBase
+        PodTemplate
+        ReplicaSet
+        StatefulSet
       ).map { |r| [r, default_group] }
 
       crs = cluster_resource_discoverer.crds.select(&:predeployed?).map { |cr| [cr.kind, { group: cr.group }] }

lib/krane/kubernetes_resource.rb

@@ -381,6 +381,19 @@ def use_generated_name(instance_data)
       @file = create_definition_tempfile
     end
 
+    def predeployed?
+      predeployed = krane_annotation_value("predeployed")
+      if type == "Role" || type == "RoleBinding"
+        true
+      else
+        default_to_predeployed? ? (predeployed != "false") : (predeployed == "true")
+      end
+    end
+
+    def default_to_predeployed?
+      false
+    end
+
     class Event
       EVENT_SEPARATOR = "ENDEVENT--BEGINEVENT"
       FIELD_SEPARATOR = "ENDFIELD--BEGINFIELD"

lib/krane/kubernetes_resource/config_map.rb

@@ -3,6 +3,10 @@ module Krane
   class ConfigMap < KubernetesResource
     TIMEOUT = 30.seconds
 
+    def predeployed?
+      true
+    end
+
     def deploy_succeeded?
       exists?
     end

lib/krane/kubernetes_resource/custom_resource.rb

@@ -74,6 +74,10 @@ def validate_definition(*, **)
       "Validation failed with: #{e}"
     end
 
+    def default_to_predeployed?
+      true
+    end
+
     private
 
     def kind

lib/krane/kubernetes_resource/custom_resource_definition.rb

@@ -90,6 +90,10 @@ def validate_rollout_conditions
       @rollout_conditions_validated = true
     end
 
+    def default_to_predeployed?
+      true
+    end
+
     private
 
     def names_accepted_condition

lib/krane/kubernetes_resource/ingress.rb

@@ -14,5 +14,6 @@ def deploy_succeeded?
     def deploy_failed?
       false
     end
+
   end
 end

lib/krane/kubernetes_resource/network_policy.rb

@@ -3,6 +3,10 @@ module Krane
   class NetworkPolicy < KubernetesResource
     TIMEOUT = 30.seconds
 
+    def predeployed?
+      true
+    end
+
     def status
       exists? ? "Created" : "Not Found"
     end

lib/krane/kubernetes_resource/persistent_volume_claim.rb

@@ -3,6 +3,10 @@ module Krane
   class PersistentVolumeClaim < KubernetesResource
     TIMEOUT = 5.minutes
 
+    def predeployed?
+      true
+    end
+
     def sync(cache)
       super
       @storage_classes = cache.get_all("StorageClass").map { |sc| StorageClass.new(sc) }

lib/krane/kubernetes_resource/pod.rb

@@ -9,6 +9,10 @@ class Pod < KubernetesResource
       Preempting
     )
 
+    def default_to_predeployed?
+      true
+    end
+
     attr_accessor :stream_logs
     attr_reader :definition
 

lib/krane/kubernetes_resource/resource_quota.rb

@@ -3,6 +3,10 @@ module Krane
   class ResourceQuota < KubernetesResource
     TIMEOUT = 30.seconds
 
+    def predeployed?
+      true
+    end
+
     def status
       exists? ? "In effect" : "Not Found"
     end

lib/krane/kubernetes_resource/role.rb

@@ -3,6 +3,10 @@ module Krane
   class Role < KubernetesResource
     TIMEOUT = 30.seconds
 
+    def predeployed?
+      true
+    end
+
     def status
       exists? ? "Created" : "Not Found"
     end

lib/krane/kubernetes_resource/role_binding.rb

@@ -3,6 +3,10 @@ module Krane
   class RoleBinding < KubernetesResource
     TIMEOUT = 30.seconds
 
+    def predeployed?
+      true
+    end
+
     def status
       exists? ? "Created" : "Not Found"
     end

lib/krane/kubernetes_resource/secret.rb

@@ -5,6 +5,10 @@ class Secret < KubernetesResource
     SENSITIVE_TEMPLATE_CONTENT = true
     SERVER_DRY_RUNNABLE = true
 
+    def predeployed?
+      true
+    end
+
     def status
       exists? ? "Available" : "Not Found"
     end

lib/krane/kubernetes_resource/service_account.rb

@@ -3,6 +3,10 @@ module Krane
   class ServiceAccount < KubernetesResource
     TIMEOUT = 30.seconds
 
+    def predeployed?
+      true
+    end
+
     def status
       exists? ? "Created" : "Not Found"
     end

lib/krane/resource_deployer.rb

@@ -49,7 +49,8 @@ def predeploy_priority_resources(resource_list, predeploy_sequence)
       predeploy_sequence.each do |resource_type, attributes|
         matching_resources = resource_list.select do |r|
           r.type == resource_type &&
-          (!attributes[:group] || r.group == attributes[:group])
+          (!attributes[:group] || r.group == attributes[:group]) &&
+          r.predeployed?
         end
         StatsD.client.gauge('priority_resources.count', matching_resources.size, tags: statsd_tags)
 
@@ -92,40 +93,42 @@ def deploy_resources(resources, prune: false, verify:, record_summary: true)
         logger.info("Deploying #{resource.id} (#{resource.pretty_timeout_type})")
       end
 
-      # Apply can be done in one large batch, the rest have to be done individually
-      applyables, individuals = resources.partition { |r| r.deploy_method == :apply }
-      # Prunable resources should also applied so that they can  be pruned
-      pruneable_types = @prune_allowlist.map { |t| t.split("/").last }
-      applyables += individuals.select { |r| pruneable_types.include?(r.type) && !r.deploy_method_override }
-
-      individuals.each do |individual_resource|
-        individual_resource.deploy_started_at = Time.now.utc
-        case individual_resource.deploy_method
-        when :create
-          err, status = create_resource(individual_resource)
-        when :replace
-          err, status = replace_or_create_resource(individual_resource)
-        when :replace_force
-          err, status = replace_or_create_resource(individual_resource, force: true)
-        else
-          # Fail Fast! This is a programmer mistake.
-          raise ArgumentError, "Unexpected deploy method! (#{individual_resource.deploy_method.inspect})"
-        end
+      StatsD.client.measure('sync.duration', tags: statsd_tags) do
+        # Apply can be done in one large batch, the rest have to be done individually
+        applyables, individuals = resources.partition { |r| r.deploy_method == :apply }
+        # Prunable resources should also applied so that they can  be pruned
+        pruneable_types = @prune_allowlist.map { |t| t.split("/").last }
+        applyables += individuals.select { |r| pruneable_types.include?(r.type) && !r.deploy_method_override }
+
+        individuals.each do |individual_resource|
+          individual_resource.deploy_started_at = Time.now.utc
+          case individual_resource.deploy_method
+          when :create
+            err, status = create_resource(individual_resource)
+          when :replace
+            err, status = replace_or_create_resource(individual_resource)
+          when :replace_force
+            err, status = replace_or_create_resource(individual_resource, force: true)
+          else
+            # Fail Fast! This is a programmer mistake.
+            raise ArgumentError, "Unexpected deploy method! (#{individual_resource.deploy_method.inspect})"
+          end
 
-        next if status.success?
+          next if status.success?
 
-        raise FatalDeploymentError, <<~MSG
-          Failed to replace or create resource: #{individual_resource.id}
-          #{individual_resource.sensitive_template_content? ? '<suppressed sensitive output>' : err}
-        MSG
-      end
+          raise FatalDeploymentError, <<~MSG
+            Failed to replace or create resource: #{individual_resource.id}
+            #{individual_resource.sensitive_template_content? ? '<suppressed sensitive output>' : err}
+          MSG
+        end
 
-      apply_all(applyables, prune)
+        apply_all(applyables, prune)
 
-      if verify
-        watcher = Krane::ResourceWatcher.new(resources: resources, deploy_started_at: deploy_started_at,
-          timeout: @global_timeout, task_config: @task_config, sha: @current_sha)
-        watcher.run(record_summary: record_summary)
+        if verify
+          watcher = Krane::ResourceWatcher.new(resources: resources, deploy_started_at: deploy_started_at,
+            timeout: @global_timeout, task_config: @task_config, sha: @current_sha)
+          watcher.run(record_summary: record_summary)
+        end
       end
     end
 

test/helpers/mock_resource.rb

@@ -25,6 +25,10 @@ def group
     "core"
   end
 
+  def predeployed?
+    false
+  end
+
   def pretty_timeout_type
   end
 

test/integration-serial/serial_deploy_test.rb

@@ -344,8 +344,7 @@ def test_cr_success_with_service
 
     assert_deploy_success(deploy_fixtures("crd", subset: %w(web.yml)))
 
-    refute_logs_match(/Predeploying priority resources/)
-    assert_logs_match_all([/Phase 3: Deploying all resources/])
+    assert_logs_match_all([/Phase 4: Deploying all resources/])
   ensure
     build_kubectl.run("delete", "-f", filepath, use_namespace: false, log_failure: false)
   end

test/unit/krane/resource_deployer_test.rb

@@ -66,6 +66,14 @@ def test_deploy_verify_false_no_failure_error
   def test_predeploy_priority_resources_respects_pre_deploy_list
     kind = "MockResource"
     resource = build_mock_resource
+    priority_list = { kind => { group: "not-#{resource.group}" } }
+    resource_deployer(kubectl_times: 0).predeploy_priority_resources([resource], priority_list)
+  end
+
+  def test_predeploy_priority_resources_respects_pre_deploy_list_and_predeployed_true_annotation
+    kind = "MockResource"
+    resource = build_mock_resource
+    resource.expects(:predeployed?).returns(true)
     watcher = mock("ResourceWatcher")
     watcher.expects(:run).returns(true)
     # ResourceDeployer only creates a ResourceWatcher if one or more resources
@@ -76,6 +84,18 @@ def test_predeploy_priority_resources_respects_pre_deploy_list
     resource_deployer.predeploy_priority_resources([resource], priority_list)
   end
 
+  def test_predeploy_priority_resources_respects_pre_deploy_list_and_predeployed_false_annotation
+    kind = "MockResource"
+    resource = build_mock_resource
+    resource.expects(:predeployed?).returns(false)
+    # ResourceDeployer only creates a ResourceWatcher if one or more resources
+    # are deployed. See test_predeploy_priority_resources_respects_empty_pre_deploy_list
+    # for counter example
+    Krane::ResourceWatcher.expects(:new).never
+    priority_list = { kind => { group: "core" } }
+    resource_deployer(kubectl_times: 0).predeploy_priority_resources([resource], priority_list)
+  end
+
   def test_predeploy_priority_resources_respects_empty_pre_deploy_list
     resource = build_mock_resource
     priority_list = []
@@ -98,4 +118,4 @@ def resource_deployer(kubectl_times: 1, prune_allowlist: [])
   def build_mock_resource(final_status: "success", hits_to_complete: 0, name: "web-pod")
     MockResource.new(name, hits_to_complete, final_status)
   end
-end
+end