Skip to content

Commit

Permalink
PR feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
@dturn
dturn committed Nov 11, 2019
1 parent 55171af commit 5310285
Show file tree
Hide file tree
Showing 6 changed files with 97 additions and 35 deletions.
3 changes: 2 additions & 1 deletion lib/krane/cli/global_deploy_command.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ 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 do not appear in the template dir",
"prune" => { type: :boolean, desc: "Enable deletion of resources that match"\
" the provided selector and do not appear in the template dir",
default: true },
}

Expand Down
41 changes: 28 additions & 13 deletions lib/krane/cluster_resource_discovery.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,37 +17,47 @@ def crds
end

def global_resource_kinds
@globals ||= fetch_resources(only_globals: true).map { |g| g["kind"] }
@globals ||= fetch_resources(namespaced: false).map { |g| g["kind"] }
end

def prunable_resources
def prunable_resources(namespaced:)
black_list = %w(Namespace Node)
api_versions = fetch_api_versions
fetch_resources.map do |resource|
next unless resource['verbs'].include?("delete")
version = api_versions.fetch(resource['apigroup'], ['v1']).last

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

# 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)
versions = { "" => %w(v1) }
if st.success?
rows = raw.split("\n")
rows.each_with_object({}) do |group_version, hash|
rows.each do |group_version|
group, version = group_version.split("/")
hash[group] ||= []
hash[group] << version
versions[group] ||= []
versions[group] << version
end
else
{}
end
versions
end

def fetch_resources(only_globals: false)
# 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=false" if only_globals
command << "--namespaced=#{namespaced}"
raw, _, st = kubectl.run(*command, output: "wide", attempts: 5,
use_namespace: false)
if st.success?
Expand All @@ -59,10 +69,15 @@ def fetch_resources(only_globals: false)
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|
thing = fields.map { |k, (s, e)| [k.strip, resource[s..e].strip] }.to_h
thing["verbs"] = thing["verbs"][1..-2].split # turn "[1 2 3]" into %w(1 2 3)
thing
end
else
[]
end
Expand Down
5 changes: 1 addition & 4 deletions lib/krane/global_deploy_task.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -197,10 +197,7 @@ def kubeclient_builder
end

def prune_whitelist
black_list = %w(Namespace Node)
cluster_resource_discoverer.prunable_resources.select do |gvk|
global_kinds.any? { |g| gvk.include?(g) } && black_list.none? { |b| gvk.include?(b) }
end
cluster_resource_discoverer.prunable_resources(namespaced: false)
end

def check_initial_status(resources)
Expand Down
8 changes: 8 additions & 0 deletions test/integration-serial/serial_deploy_test.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -604,21 +604,29 @@ def test_global_deploy_validation_catches_namespaced_cr
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
Expand Down
16 changes: 7 additions & 9 deletions test/integration/global_deploy_test.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ def test_global_deploy_task_success
%r{Assuming StorageClass\/testing-storage-class[\w-]+ deployed successfully.},
%r{Successfully deployed in [\d.]+s: PriorityClass/testing-priority-class[\w-]+, StorageClass\/testing-storage-},
"Result: SUCCESS",
"Successfully deployed 2 resource",
"Successfully deployed 2 resources",
"Successful resources",
"StorageClass/testing-storage-class",
"PriorityClass/testing-priority-class",
Expand Down Expand Up @@ -66,7 +66,7 @@ def test_global_deploy_task_success_verify_false
%r{StorageClass\/testing-storage-class[\w-]+ \(timeout: 300s\)},
%r{PriorityClass/testing-priority-class[\w-]+ \(timeout: 300s\)},
"Result: SUCCESS",
"Deployed 2 resource",
"Deployed 2 resources",
"Deploy result verification is disabled for this deploy.",
"This means the desired changes were communicated to Kubernetes, but the"\
" deploy did not make sure they actually succeeded.",
Expand Down Expand Up @@ -103,7 +103,7 @@ def test_global_deploy_task_success_selector
%r{Assuming StorageClass\/testing-storage-class[\w-]+ deployed successfully.},
/Successfully deployed in [\d.]+s/,
"Result: SUCCESS",
"Successfully deployed 2 resource",
"Successfully deployed 2 resources",
"Successful resources",
"StorageClass/testing-storage-class",
"PriorityClass/testing-priority-class",
Expand All @@ -127,7 +127,7 @@ def test_global_deploy_task_failure
])
end

def test_global_deploy_prune_black_box_success
def test_global_deploy_prune_success
assert_deploy_success(deploy_global_fixtures('globals', clean_up: false, selector: 'test=prune1'))
reset_logger
assert_deploy_success(deploy_global_fixtures('globals', subset: 'storage_classes.yml', selector: 'test=prune1'))
Expand All @@ -141,8 +141,7 @@ def test_global_deploy_prune_black_box_success
%r{StorageClass\/testing-storage-class[\w-]+\s+Exists},
"Phase 3: Deploying all resources",
%r{Deploying StorageClass\/testing-storage-class[\w-]+ \(timeout: 300s\)},
"Don't know how to monitor resources of type StorageClass.",
%r{Assuming StorageClass\/testing-storage-class[\w-]+ deployed successfully.},
"The following resources were pruned: priorityclass.scheduling.k8s.io/testing-priority-class",
%r{Successfully deployed in [\d.]+s: StorageClass\/testing-storage-class},
"Result: SUCCESS",
"Pruned 1 resource and successfully deployed 1 resource",
Expand All @@ -151,7 +150,7 @@ def test_global_deploy_prune_black_box_success
])
end

def test_no_prune_global_deploy_black_box_success
def test_no_prune_global_deploy_success
assert_deploy_success(deploy_global_fixtures('globals', clean_up: false, selector: 'test=prune2'))
reset_logger
assert_deploy_success(deploy_global_fixtures('globals', subset: 'storage_classes.yml',
Expand All @@ -166,14 +165,13 @@ def test_no_prune_global_deploy_black_box_success
%r{StorageClass\/testing-storage-class[\w-]+\s+Exists},
"Phase 3: Deploying all resources",
%r{Deploying StorageClass\/testing-storage-class[\w-]+ \(timeout: 300s\)},
"Don't know how to monitor resources of type StorageClass.",
%r{Assuming StorageClass\/testing-storage-class[\w-]+ deployed successfully.},
%r{Successfully deployed in [\d.]+s: StorageClass\/testing-storage-class},
"Result: SUCCESS",
"Successfully deployed 1 resource",
"Successful resources",
"StorageClass/testing-storage-class",
])
refute_logs_match(/[pP]runed/)
assert_deploy_success(deploy_global_fixtures('globals', selector: 'test=prune2'))
end
end
59 changes: 51 additions & 8 deletions test/unit/cluster_resource_discovery_test.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,32 +9,75 @@ def test_global_resource_kinds_failure
end

def test_global_resource_kinds_success
crd = mocked_cluster_resource_discovery(full_response)
crd = mocked_cluster_resource_discovery(api_resources_full_response)
kinds = crd.global_resource_kinds
assert_equal(kinds.length, full_response.split("\n").length - 1)
assert_equal(kinds.length, api_resources_full_response.split("\n").length - 1)
%w(MutatingWebhookConfiguration ComponentStatus CustomResourceDefinition).each do |kind|
assert_includes(kinds, kind)
end
end

def test_prunable_resources
crd = mocked_cluster_resource_discovery(full_response)
kinds = crd.prunable_resources
assert_equal(kinds.length, 15)
%w(scheduling.k8s.io/v1/PriorityClass storage.k8s.io/v1/StorageClass).each do |kind|
Krane::Kubectl.any_instance.stubs(:run).with("api-versions", attempts: 5, use_namespace: false)
.returns([api_versions_full_response, "", stub(success?: true)])
crd = mocked_cluster_resource_discovery(api_resources_full_response)
kinds = crd.prunable_resources(namespaced: false)

assert_equal(kinds.length, 13)
%w(scheduling.k8s.io/v1beta1/PriorityClass storage.k8s.io/v1beta1/StorageClass).each do |kind|
assert_includes(kinds, kind)
end
%W(node namespace).each do |black_lised_kind|
assert_empty kinds.select { |k| k.downcase.include?(black_lised_kind) }
end
end

private

def mocked_cluster_resource_discovery(response, success: true)
Krane::Kubectl.any_instance.stubs(:run).returns([response, "", stub(success?: success)])
Krane::Kubectl.any_instance.stubs(:run)
.with("api-resources", "--namespaced=false", attempts: 5, use_namespace: false, output: "wide")
.returns([response, "", stub(success?: success)])
Krane::ClusterResourceDiscovery.new(task_config: task_config, namespace_tags: [])
end

def api_versions_full_response
%(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)
end

# rubocop:disable Metrics/LineLength
def full_response
def api_resources_full_response
%(NAME SHORTNAMES APIGROUP NAMESPACED KIND VERBS
componentstatuses cs false ComponentStatus [get list]
namespaces ns false Namespace [create delete get list patch update watch]
Expand Down

0 comments on commit 5310285

Please sign in to comment.