Global prune 2 (#612)

* Prune global resources
* Add code comments for unexpected behaviors

lib/krane/cli/global_deploy_command.rb

@@ -14,6 +14,9 @@ class GlobalDeployCommand
                              desc: "Verify workloads correctly deployed" },
         "selector" => { type: :string, banner: "'label=value'", required: true,
                         desc: "Select workloads owned by selector(s)" },
+        "prune" => { type: :boolean, desc: "Enable deletion of resources that match"\
+                     " the provided selector and do not appear in the provided templates",
+                     default: true },
       }
 
       def self.from_options(context, options)
@@ -35,7 +38,7 @@ def self.from_options(context, options)
 
           deploy.run!(
             verify_result: options["verify-result"],
-            prune: false,
+            prune: options[:prune],
           )
         end
       end

lib/krane/cluster_resource_discovery.rb

@@ -17,13 +17,49 @@ def crds
     end
 
     def global_resource_kinds
-      @globals ||= fetch_globals.map { |g| g["kind"] }
+      @globals ||= fetch_resources(namespaced: false).map { |g| g["kind"] }
+    end
+
+    def prunable_resources(namespaced:)
+      black_list = %w(Namespace Node)
+      api_versions = fetch_api_versions
+
+      fetch_resources(namespaced: namespaced).map do |resource|
+        next unless resource['verbs'].one? { |v| v == "delete" }
+        next if black_list.include?(resource['kind'])
+        version = api_versions[resource['apigroup'].to_s].last
+        [resource['apigroup'], version, resource['kind']].compact.join("/")
+      end.compact
     end
 
     private
 
-    def fetch_globals
-      raw, _, st = kubectl.run("api-resources", "--namespaced=false", output: "wide", attempts: 5,
+    # kubectl api-versions returns a list of group/version strings e.g. autoscaling/v2beta2
+    # A kind may not exist in all versions of the group.
+    def fetch_api_versions
+      raw, _, st = kubectl.run("api-versions", attempts: 5, use_namespace: false)
+      # The "core" group is represented by an empty string
+      versions = { "" => %w(v1) }
+      if st.success?
+        rows = raw.split("\n")
+        rows.each do |group_version|
+          group, version = group_version.split("/")
+          versions[group] ||= []
+          versions[group] << version
+        end
+      end
+      versions
+    end
+
+    # kubectl api-resources -o wide returns 5 columns
+    # NAME SHORTNAMES APIGROUP NAMESPACED KIND VERBS
+    # SHORTNAMES and APIGROUP may be blank
+    # VERBS is an array
+    # serviceaccounts sa <blank> true ServiceAccount [create delete deletecollection get list patch update watch]
+    def fetch_resources(namespaced: false)
+      command = %w(api-resources)
+      command << "--namespaced=#{namespaced}"
+      raw, _, st = kubectl.run(*command, output: "wide", attempts: 5,
         use_namespace: false)
       if st.success?
         rows = raw.split("\n")
@@ -34,9 +70,16 @@ def fetch_globals
         fields = full_width_field_names.each_with_object({}) do |name, hash|
           start = cursor
           cursor = start + name.length
+          # Last field should consume the remainder of the line
+          cursor = 0 if full_width_field_names.last == name.strip
           hash[name.strip] = [start, cursor - 1]
         end
-        resources.map { |r| fields.map { |k, (s, e)| [k.strip, r[s..e].strip] }.to_h }
+        resources.map do |resource|
+          resource = fields.map { |k, (s, e)| [k.strip, resource[s..e].strip] }.to_h
+          # Manually parse verbs: "[get list]" into %w(get list)
+          resource["verbs"] = resource["verbs"][1..-2].split
+          resource
+        end
       else
         []
       end

lib/krane/global_deploy_task.rb

@@ -103,7 +103,6 @@ def run!(verify_result: true, prune: true)
     private
 
     def deploy!(resources, verify_result, prune)
-      prune_whitelist = []
       resource_deployer = ResourceDeployer.new(task_config: @task_config,
         prune_whitelist: prune_whitelist, max_watch_seconds: @global_timeout,
         selector: @selector, statsd_tags: statsd_tags)
@@ -197,6 +196,10 @@ def kubeclient_builder
       @kubeclient_builder ||= KubeclientBuilder.new
     end
 
+    def prune_whitelist
+      cluster_resource_discoverer.prunable_resources(namespaced: false)
+    end
+
     def check_initial_status(resources)
       cache = ResourceCache.new(@task_config)
       Concurrency.split_across_threads(resources) { |r| r.sync(cache) }

lib/krane/kubeclient_builder.rb

@@ -103,6 +103,14 @@ def build_storage_v1_kubeclient(context)
       )
     end
 
+    def build_scheduling_v1beta1_kubeclient(context)
+      build_kubeclient(
+        api_version: "v1beta1",
+        context: context,
+        endpoint_path: "/apis/scheduling.k8s.io"
+      )
+    end
+
     def validate_config_files
       errors = []
       if @kubeconfig_files.empty?

test/exe/global_deploy_test.rb

@@ -36,6 +36,11 @@ def test_deploy_parses_selector
     krane_global_deploy!(flags: "--selector #{selector}")
   end
 
+  def test_deploy_parses_prune
+    set_krane_global_deploy_expectations!(run_args: { prune: false })
+    krane_global_deploy!(flags: '--prune false')
+  end
+
   private
 
   def set_krane_global_deploy_expectations!(new_args: {}, run_args: {})
@@ -68,7 +73,7 @@ def default_options(new_args = {}, run_args = {})
       }.merge(new_args),
       run_args: {
         verify_result: true,
-        prune: false,
+        prune: true,
       }.merge(run_args),
     }
   end

test/fixtures/for_unit_tests/api_resources.txt

@@ -0,0 +1,21 @@
+NAME                              SHORTNAMES   APIGROUP                       NAMESPACED   KIND                             VERBS
+componentstatuses                 cs                                          false        ComponentStatus                  [get list]
+namespaces                        ns                                          false        Namespace                        [create delete get list patch update watch]
+nodes                             no                                          false        Node                             [create delete deletecollection get list patch update watch]
+persistentvolumes                 pv                                          false        PersistentVolume                 [create delete deletecollection get list patch update watch]
+mutatingwebhookconfigurations                  admissionregistration.k8s.io   false        MutatingWebhookConfiguration     [create delete deletecollection get list patch update watch]
+validatingwebhookconfigurations                admissionregistration.k8s.io   false        ValidatingWebhookConfiguration   [create delete deletecollection get list patch update watch]
+customresourcedefinitions         crd,crds     apiextensions.k8s.io           false        CustomResourceDefinition         [create delete deletecollection get list patch update watch]
+apiservices                                    apiregistration.k8s.io         false        APIService                       [create delete deletecollection get list patch update watch]
+tokenreviews                                   authentication.k8s.io          false        TokenReview                      [create]
+selfsubjectaccessreviews                       authorization.k8s.io           false        SelfSubjectAccessReview          [create]
+selfsubjectrulesreviews                        authorization.k8s.io           false        SelfSubjectRulesReview           [create]
+subjectaccessreviews                           authorization.k8s.io           false        SubjectAccessReview              [create]
+certificatesigningrequests        csr          certificates.k8s.io            false        CertificateSigningRequest        [create delete deletecollection get list patch update watch]
+podsecuritypolicies               psp          extensions                     false        PodSecurityPolicy                [create delete deletecollection get list patch update watch]
+podsecuritypolicies               psp          policy                         false        PodSecurityPolicy                [create delete deletecollection get list patch update watch]
+clusterrolebindings                            rbac.authorization.k8s.io      false        ClusterRoleBinding               [create delete deletecollection get list patch update watch]
+clusterroles                                   rbac.authorization.k8s.io      false        ClusterRole                      [create delete deletecollection get list patch update watch]
+priorityclasses                   pc           scheduling.k8s.io              false        PriorityClass                    [create delete deletecollection get list patch update watch]
+storageclasses                    sc           storage.k8s.io                 false        StorageClass                     [create delete deletecollection get list patch update watch]
+volumeattachments                              storage.k8s.io                 false        VolumeAttachment                 [create delete deletecollection get list patch update watch]

test/fixtures/for_unit_tests/api_versions.txt

@@ -0,0 +1,32 @@
+admissionregistration.k8s.io/v1
+admissionregistration.k8s.io/v1beta1
+apiextensions.k8s.io/v1
+apiextensions.k8s.io/v1beta1
+apiregistration.k8s.io/v1
+apiregistration.k8s.io/v1beta1
+apps/v1
+authentication.k8s.io/v1
+authentication.k8s.io/v1beta1
+authorization.k8s.io/v1
+authorization.k8s.io/v1beta1
+autoscaling/v1
+autoscaling/v2beta1
+autoscaling/v2beta2
+batch/v1
+batch/v1beta1
+certificates.k8s.io/v1beta1
+coordination.k8s.io/v1
+coordination.k8s.io/v1beta1
+events.k8s.io/v1beta1
+extensions/v1beta1
+networking.k8s.io/v1
+networking.k8s.io/v1beta1
+node.k8s.io/v1beta1
+policy/v1beta1
+rbac.authorization.k8s.io/v1
+rbac.authorization.k8s.io/v1beta1
+scheduling.k8s.io/v1
+scheduling.k8s.io/v1beta1
+storage.k8s.io/v1
+storage.k8s.io/v1beta1
+v1

test/fixtures/globals/priority_class.yml

@@ -0,0 +1,8 @@
+apiVersion: scheduling.k8s.io/v1beta1
+description: Used for testing global deploys and pruning.
+kind: PriorityClass
+metadata:
+  name: testing-priority-class
+  labels:
+    app: krane
+value: 20

test/helpers/fixture_deploy_helper.rb

@@ -46,7 +46,7 @@ def deploy_global_fixtures(set, subset: nil, **args)
     fixtures = load_fixtures(set, subset)
     raise "Cannot deploy empty template set" if fixtures.empty?
     args[:selector] ||= "test=#{@namespace}"
-    namespace_globals(fixtures)
+    namespace_globals(fixtures, args[:selector])
 
     yield fixtures if block_given?
 
@@ -104,7 +104,7 @@ def deploy_dirs_without_profiling(dirs, wait: true, allow_protected_ns: false, p
     )
   end
 
-  def global_deploy_dirs_without_profiling(dirs, verify_result: true, prune: false,
+  def global_deploy_dirs_without_profiling(dirs, clean_up: true, verify_result: true, prune: true,
     global_timeout: 300, selector:)
     deploy = Krane::GlobalDeployTask.new(
       context: KubeclientHelper::TEST_CONTEXT,
@@ -118,7 +118,7 @@ def global_deploy_dirs_without_profiling(dirs, verify_result: true, prune: false
       prune: prune
     )
   ensure
-    delete_globals(Array(dirs))
+    delete_globals(Array(dirs)) if clean_up
   end
 
   # Deploys all fixtures in the given directories via KubernetesDeploy::DeployTask
@@ -180,13 +180,15 @@ def build_kubectl(log_failure_by_default: true, timeout: '5s')
       log_failure_by_default: log_failure_by_default, default_timeout: timeout)
   end
 
-  def namespace_globals(fixtures)
+  def namespace_globals(fixtures, selector)
+    selector_key, selector_value = selector.split("=")
     fixtures.each do |_, kinds_map|
       kinds_map.each do |_, resources|
         resources.each do |resource|
           resource["metadata"]["name"] = (resource["metadata"]["name"] + @namespace)[0..63]
+          resource["metadata"]["name"] += "0" if resource["metadata"]["name"].end_with?("-")
           resource["metadata"]["labels"] ||= {}
-          resource["metadata"]["labels"]["test"] = @namespace
+          resource["metadata"]["labels"][selector_key] = selector_value
         end
       end
     end

test/helpers/kubeclient_helper.rb

@@ -52,6 +52,10 @@ def storage_v1_kubeclient
     @storage_v1_kubeclient ||= kubeclient_builder.build_storage_v1_kubeclient(TEST_CONTEXT)
   end
 
+  def scheduling_v1beta1_kubeclient
+    @scheduling_v1beta1_kubeclient ||= kubeclient_builder.build_scheduling_v1beta1_kubeclient(TEST_CONTEXT)
+  end
+
   def kubeclient_builder
     @kubeclient_builder ||= Krane::KubeclientBuilder.new
   end

test/integration-serial/serial_deploy_test.rb

@@ -603,6 +603,32 @@ def test_global_deploy_validation_catches_namespaced_cr
     wait_for_all_crd_deletion
   end
 
+  def test_global_deploy_prune_black_box_success
+    namespace_name = "test-app"
+    setup_template_dir("globals") do |target_dir|
+      flags = "-f #{target_dir} --selector app=krane"
+      namespace_str = "apiVersion: v1\nkind: Namespace\nmetadata:\n  name: #{namespace_name}"\
+      "\n  labels:\n    app: krane"
+      File.write(File.join(target_dir, "namespace.yml"), namespace_str)
+      out, err, status = krane_black_box("global-deploy", "#{KubeclientHelper::TEST_CONTEXT} #{flags}")
+      assert_empty(out)
+      assert_match("Successfully deployed 3 resource", err)
+      assert_match(/#{namespace_name}\W+Exists/, err)
+      assert_match("Success", err)
+      assert_predicate(status, :success?)
+
+      flags = "-f #{target_dir}/storage_classes.yml --selector app=krane"
+      out, err, status = krane_black_box("global-deploy", "#{KubeclientHelper::TEST_CONTEXT} #{flags}")
+      assert_empty(out)
+      refute_match(namespace_name, err) # Asserting that the namespace is not pruned
+      assert_match("Pruned 1 resource and successfully deployed 1 resource", err)
+      assert_predicate(status, :success?)
+    end
+  ensure
+    build_kubectl.run("delete", "-f", fixture_path("globals"), use_namespace: false, log_failure: false)
+    build_kubectl.run("delete", "namespace", namespace_name, use_namespace: false, log_failure: false)
+  end
+
   private
 
   def wait_for_all_crd_deletion