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.

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

Batched dry-run? No thank you! #946

Merged
merged 6 commits into from
Jan 30, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
9 changes: 7 additions & 2 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ jobs:
ruby-tests:
runs-on: ubuntu-latest

name: "Tests - Ruby ${{ matrix.ruby }} with Kubernetes ${{ matrix.kubernetes_version }}"
name: "Tests (${{matrix.test_suite}}) - Ruby ${{ matrix.ruby }} with Kubernetes ${{ matrix.kubernetes_version }}"
strategy:
fail-fast: false
matrix:
Expand All @@ -20,6 +20,11 @@ jobs:
- "1.26.4"
- "1.24.13"
- "1.23.17"
test_suite:
- "unit_test"
- "cli_test"
- "serial_integration_test"
- "integration_test"
include:
- kubernetes_version: "1.27.3"
kind_image: "kindest/node:v1.27.3@sha256:9dd3392d79af1b084671b05bcf65b21de476256ad1dcc853d9f3b10b4ac52dde"
Expand Down Expand Up @@ -52,4 +57,4 @@ jobs:

- name: Run tests
run: |
bin/test
bin/test ${{matrix.test_suite}}
23 changes: 15 additions & 8 deletions bin/test
Original file line number Diff line number Diff line change
Expand Up @@ -33,16 +33,23 @@ if [[ "${CI:-0}" != "0" ]]; then
PARALLELISM=2
fi

print_header "Run CLI Tests"
bundle exec rake cli_test
test_type=$1

print_header "Run Unit Tests"
bundle exec rake unit_test

print_header "Run Non-Parallel Integration Tests"
bundle exec rake serial_integration_test
case $test_type in
cli_test | unit_test | serial_integration_test)
print_header $test_type
bundle exec rake $test_type
;;

print_header "Run Parallel Integration Tests (MT_CPU=$PARALLELISM)"
PARALLELIZE_ME=1 MT_CPU=$PARALLELISM bundle exec rake integration_test
integration_test)
print_header "Run Parallel Integration Tests (MT_CPU=$PARALLELISM)"
PARALLELIZE_ME=1 MT_CPU=$PARALLELISM bundle exec rake integration_test
;;

*)
echo "Argument must be one of: unit_test, cli_test, serial_integration_test, integration_test"
;;
esac

test $err -eq 0
12 changes: 1 addition & 11 deletions lib/krane/deploy_task.rb
Original file line number Diff line number Diff line change
Expand Up @@ -287,15 +287,9 @@ def validate_configuration(prune:)

def validate_resources(resources)
validate_globals(resources)
batch_dry_run_success = @skip_dry_run || validate_dry_run(resources)
resources.select! { |r| r.selected?(@selector) } if @selector_as_filter
Krane::Concurrency.split_across_threads(resources) do |r|
# No need to pass in kubectl (and do per-resource dry run apply) if batch dry run succeeded
if batch_dry_run_success
r.validate_definition(kubectl: nil, selector: @selector, dry_run: false)
else
r.validate_definition(kubectl: kubectl, selector: @selector, dry_run: true)
end
r.validate_definition(kubectl: kubectl, selector: @selector, dry_run: true)
end
failed_resources = resources.select(&:validation_failed?)
if failed_resources.present?
Expand All @@ -321,10 +315,6 @@ def validate_globals(resources)
"Use GlobalDeployTask instead."
end

def validate_dry_run(resources)
resource_deployer.dry_run(resources)
end

def namespace_definition
@namespace_definition ||= begin
definition, _err, st = kubectl.run("get", "namespace", @namespace, use_namespace: false,
Expand Down
7 changes: 0 additions & 7 deletions lib/krane/resource_deployer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,6 @@ def initialize(task_config:, prune_allowlist:, global_timeout:, current_sha: nil
@statsd_tags = statsd_tags
end

def dry_run(resources)
apply_all(resources, true, dry_run: true)
true
rescue FatalDeploymentError
false
end

def deploy!(resources, verify_result, prune)
if verify_result
deploy_all_resources(resources, prune: prune, verify: true)
Expand Down
20 changes: 0 additions & 20 deletions test/integration-serial/serial_deploy_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -526,26 +526,6 @@ def test_resource_discovery_stops_deploys_when_fetch_crds_kubectl_errs
], in_order: true)
end

def test_batch_dry_run_apply_failure_falls_back_to_individual_resource_dry_run_validation
Krane::KubernetesResource.any_instance.expects(:validate_definition).with do |kwargs|
kwargs[:kubectl].is_a?(Krane::Kubectl) && kwargs[:dry_run]
end
deploy_fixtures("hello-cloud", subset: %w(secret.yml)) do |fixtures|
secret = fixtures["secret.yml"]["Secret"].first
secret["bad_field"] = "bad_key"
end
end

def test_batch_dry_run_apply_success_precludes_individual_resource_dry_run_validation
Krane::KubernetesResource.any_instance.expects(:validate_definition).with { |params| params[:dry_run] == false }
result = deploy_fixtures("hello-cloud", subset: %w(secret.yml))
assert_deploy_success(result)
assert_logs_match_all([
"Result: SUCCESS",
"Successfully deployed 1 resource",
], in_order: true)
end

def test_skip_dry_run_apply_success
Krane::KubernetesResource.any_instance.expects(:validate_definition).with { |params| params[:dry_run] == false }
Krane::ResourceDeployer.any_instance.expects(:dry_run).never
Expand Down
Loading