Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Global prune 2 #612

Merged
merged 4 commits into from
Nov 12, 2019
Merged

Global prune 2 #612

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
cb4e754
Prune global resources
dturn Oct 24, 2019
55171af
udpate tests
dturn Nov 4, 2019
afcff78
PR feedback
dturn Nov 11, 2019
5fb93cb
More code comments
dturn Nov 11, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion lib/krane/cli/global_deploy_command.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -35,7 +38,7 @@ def self.from_options(context, options)

deploy.run!(
verify_result: options["verify-result"],
prune: false,
prune: options[:prune],
)
end
end
Expand Down
51 changes: 47 additions & 4 deletions lib/krane/cluster_resource_discovery.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
dturn marked this conversation as resolved.
Show resolved Hide resolved
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("/")
dturn marked this conversation as resolved.
Show resolved Hide resolved
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")
Expand All @@ -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
dturn marked this conversation as resolved.
Show resolved Hide resolved
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
Expand Down
5 changes: 4 additions & 1 deletion lib/krane/global_deploy_task.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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) }
Expand Down
8 changes: 8 additions & 0 deletions lib/krane/kubeclient_builder.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,14 @@ def build_storage_v1_kubeclient(context)
)
end

def build_scheduling_v1beta1_kubeclient(context)
Copy link
Contributor

@dirceu dirceu Nov 12, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this actually necessary? I tried removing this and running tests but they keep passing; I tested with the tests modified/added in this PR:

$ alias rt="bundle exec ruby -I test"
$ rt test/integration-serial/serial_deploy_test.rb --name test_global_deploy_prune_black_box_success
$ rt test/integration/global_deploy_test.rb
$ rt test/unit/cluster_resource_discovery_test.rb

Am I missing anything?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

build_scheduling_v1beta1_kubeclient are mostly used in the tests to allow for manual interaction with resources, usually to delete them in an ensure block. They aren't being used but seemed less than ideal to add a new kind to the fixtures but not add its helper methods.

build_kubeclient(
api_version: "v1beta1",
context: context,
endpoint_path: "/apis/scheduling.k8s.io"
)
end

def validate_config_files
errors = []
if @kubeconfig_files.empty?
Expand Down
7 changes: 6 additions & 1 deletion test/exe/global_deploy_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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: {})
Expand Down Expand Up @@ -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
Expand Down
21 changes: 21 additions & 0 deletions test/fixtures/for_unit_tests/api_resources.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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]
32 changes: 32 additions & 0 deletions test/fixtures/for_unit_tests/api_versions.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
8 changes: 8 additions & 0 deletions test/fixtures/globals/priority_class.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
12 changes: 7 additions & 5 deletions test/helpers/fixture_deploy_helper.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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?

Expand Down Expand Up @@ -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,
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions test/helpers/kubeclient_helper.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
26 changes: 26 additions & 0 deletions test/integration-serial/serial_deploy_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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|
dturn marked this conversation as resolved.
Show resolved Hide resolved
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?)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

flush the logger

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

krane_black_box uses Open3.capture3 so each call get's a unique stream

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
Expand Down
Loading