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

Batch dry run but like for real this time #943

Closed
Closed
Changes from 3 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
11 changes: 7 additions & 4 deletions lib/krane/deploy_task.rb
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,6 @@ def run(**args)
def run!(verify_result: true, prune: true)
start = Time.now.utc
@logger.reset

@logger.phase_heading("Initializing deploy")
validate_configuration(prune: prune)
resources = discover_resources
Expand Down Expand Up @@ -285,17 +284,21 @@ def validate_configuration(prune:)
measure_method(:validate_configuration)

def validate_resources(resources)
validate_globals(resources)
batch_dry_run_success = validate_dry_run(resources)
resources.select! { |r| r.selected?(@selector) } if @selector_as_filter
Krane::Concurrency.split_across_threads(resources) do |r|
validate_globals(resources)
applyables, individuals = resources.partition { |r| r.deploy_method == :apply }
batch_dry_run_success = validate_dry_run(applyables)
resources_to_validate = batch_dry_run_success ? individuals : resources

Krane::Concurrency.split_across_threads(resources_to_validate) do |r|
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Originally I wanted to to just trust the output of the batched dry run test and raise a FatalDeploymentError. However, that made it very difficult to give a consistent UX with regards to giving useful feedback: the main issue being it became necessary to suppress all feedback if a sensitive document/secret was contained in the apply set.

To that end, I've kept things pretty similar, just removing resources from the per-resource checks if batched dry-run succeeds. If it fails, since that's a less common case, we default to per-resource checking in order to provide more granular feedback and highlight what the actual issue it. This is the difference between "Something somewhere went wrong" and "THIS particular file and THIS particular line are wrong".

# 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
end

failed_resources = resources.select(&:validation_failed?)
if failed_resources.present?
failed_resources.each do |r|
Expand Down
Loading