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

Refactor/update attempts kubectl apply #751

Merged
merged 29 commits into from
Oct 5, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
85a68ca
Add retries to certain kubectl calls
gigr Sep 10, 2020
794868e
added attempts change and logs
ruionweb Sep 10, 2020
1623614
rubocop correction
ruionweb Sep 10, 2020
e9c175e
added tests
ruionweb Sep 11, 2020
1f9537c
changed testing attempts
ruionweb Sep 11, 2020
4263ef5
increasing attempts for kubectl apply
ruionweb Sep 11, 2020
6fa9187
rubocop
ruionweb Sep 11, 2020
17359b7
removing space
ruionweb Sep 24, 2020
ba2aba6
empty commit
timothysmith0609 Sep 24, 2020
d582ea2
Updated rubocop using bundlelock.
ajshepley Sep 25, 2020
31a1ec2
Update gemspec.
ajshepley Sep 25, 2020
e03084a
Ran rubocop -a to autocorrect issues.
ajshepley Sep 25, 2020
5b3022c
Ignore callSuper warning for test classes extending fixtureset.
ajshepley Sep 25, 2020
683801c
Send definition to superclass for kubernetes_resource subclass.
ajshepley Sep 25, 2020
a9521c1
Merge pull request #752 from Shopify/ajs/update_rubocop
ajshepley Sep 25, 2020
2ee12e1
Disable warnings when running tests
DazWorrall Sep 7, 2020
f3fd4dc
Allow specifying a kubeconfig on deploy tasks
DazWorrall Sep 7, 2020
4f32bcf
Allow configurable kubeconfig on other tasks
DazWorrall Sep 7, 2020
5210675
Delegate kubeclient_builder to task config class.
ajshepley Sep 10, 2020
fb97d04
Remove warning stubs.
ajshepley Sep 10, 2020
858863f
Moved Kubeconfig task test to TaskConfig test.
ajshepley Sep 10, 2020
06f3428
Add explicit getter for kubeconfig ivar.
ajshepley Sep 21, 2020
fa18cc1
Added test lines for retrieving kubeconfig builder.
ajshepley Sep 24, 2020
4239cde
Merge pull request #746 from Shopify/internal-api-kubeconfig
ajshepley Sep 25, 2020
fc30f27
Merge pull request #749 from Shopify/refactor/attempts-number-and-logs
ruionweb Oct 5, 2020
6e0673a
increasing attempts for kubectl apply
ruionweb Sep 11, 2020
207f0f1
rubocop
ruionweb Sep 11, 2020
42a1fe3
removing space
ruionweb Sep 24, 2020
f2bbc88
updating branch
ruionweb Oct 5, 2020
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
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ Style/MethodCallWithArgsParentheses:
- raise
- puts
Exclude:
- Gemfile
- '**/Gemfile'

Style/MethodDefParentheses:
EnforcedStyle: require_parentheses
Expand Down Expand Up @@ -675,7 +675,7 @@ Style/LineEndConcatenation:
Style/MethodCallWithoutArgsParentheses:
Enabled: true

Style/MethodMissingSuper:
Lint/MissingSuper:
Enabled: true

Style/MissingRespondToMissing:
Expand Down Expand Up @@ -965,7 +965,7 @@ Lint/UselessAccessModifier:
Lint/UselessAssignment:
Enabled: true

Lint/UselessComparison:
Lint/BinaryOperatorWithIdenticalOperands:
Enabled: true

Lint/UselessElseWithoutRescue:
Expand Down
2 changes: 1 addition & 1 deletion krane.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,6 @@ Gem::Specification.new do |spec|
spec.add_development_dependency("byebug")
spec.add_development_dependency("ruby-prof")
spec.add_development_dependency("ruby-prof-flamegraph")
spec.add_development_dependency("rubocop", "~> 0.88.0")
spec.add_development_dependency("rubocop", "~> 0.89.1")
spec.add_development_dependency("codecov")
end
12 changes: 6 additions & 6 deletions lib/krane/deploy_task.rb
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,10 @@ def server_version
kubectl.server_version
end

attr_reader :task_config

delegate :kubeclient_builder, to: :task_config

# Initializes the deploy task
#
# @param namespace [String] Kubernetes namespace (*required*)
Expand All @@ -101,10 +105,10 @@ def server_version
# @param render_erb [Boolean] Enable ERB rendering
def initialize(namespace:, context:, current_sha: nil, logger: nil, kubectl_instance: nil, bindings: {},
global_timeout: nil, selector: nil, filenames: [], protected_namespaces: nil,
render_erb: false)
render_erb: false, kubeconfig: nil)
@logger = logger || Krane::FormattedLogger.build(namespace, context)
@template_sets = TemplateSets.from_dirs_and_files(paths: filenames, logger: @logger, render_erb: render_erb)
@task_config = Krane::TaskConfig.new(context, namespace, @logger)
@task_config = Krane::TaskConfig.new(context, namespace, @logger, kubeconfig)
@bindings = bindings
@namespace = namespace
@namespace_tags = []
Expand Down Expand Up @@ -190,10 +194,6 @@ def resource_deployer
selector: @selector, statsd_tags: statsd_tags, current_sha: @current_sha)
end

def kubeclient_builder
@kubeclient_builder ||= KubeclientBuilder.new
end

def cluster_resource_discoverer
@cluster_resource_discoverer ||= ClusterResourceDiscovery.new(
task_config: @task_config,
Expand Down
11 changes: 4 additions & 7 deletions lib/krane/global_deploy_task.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,18 +25,19 @@ module Krane
class GlobalDeployTask
extend Krane::StatsD::MeasureMethods
include TemplateReporting
delegate :context, :logger, :global_kinds, to: :@task_config
delegate :context, :logger, :global_kinds, :kubeclient_builder, to: :@task_config
attr_reader :task_config

# Initializes the deploy task
#
# @param context [String] Kubernetes context (*required*)
# @param global_timeout [Integer] Timeout in seconds
# @param selector [Hash] Selector(s) parsed by Krane::LabelSelector (*required*)
# @param filenames [Array<String>] An array of filenames and/or directories containing templates (*required*)
def initialize(context:, global_timeout: nil, selector: nil, filenames: [], logger: nil)
def initialize(context:, global_timeout: nil, selector: nil, filenames: [], logger: nil, kubeconfig: nil)
template_paths = filenames.map { |path| File.expand_path(path) }

@task_config = TaskConfig.new(context, nil, logger)
@task_config = TaskConfig.new(context, nil, logger, kubeconfig)
@template_sets = TemplateSets.from_dirs_and_files(paths: template_paths,
logger: @task_config.logger, render_erb: false)
@global_timeout = global_timeout
Expand Down Expand Up @@ -189,10 +190,6 @@ def kubectl
@kubectl ||= Kubectl.new(task_config: @task_config, log_failure_by_default: true)
end

def kubeclient_builder
@kubeclient_builder ||= KubeclientBuilder.new
end

def prune_whitelist
cluster_resource_discoverer.prunable_resources(namespaced: false)
end
Expand Down
6 changes: 4 additions & 2 deletions lib/krane/kubeclient_builder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,10 @@ def initialize(context_name, kubeconfig)
end
end

def initialize(kubeconfig: ENV["KUBECONFIG"])
files = kubeconfig || "#{Dir.home}/.kube/config"
attr_reader :kubeconfig_files

def initialize(kubeconfig: nil)
files = kubeconfig || ENV["KUBECONFIG"] || "#{Dir.home}/.kube/config"
# Split the list by colon for Linux and Mac, and semicolon for Windows.
@kubeconfig_files = files.split(/[:;]/).map!(&:strip).reject(&:empty?)
end
Expand Down
10 changes: 6 additions & 4 deletions lib/krane/kubectl.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ class Kubectl

class ResourceNotFoundError < StandardError; end

delegate :namespace, :context, :logger, to: :@task_config
delegate :namespace, :context, :logger, :kubeconfig, to: :@task_config

def initialize(task_config:, log_failure_by_default:, default_timeout: DEFAULT_TIMEOUT,
output_is_sensitive_default: false)
Expand All @@ -34,7 +34,8 @@ def run(*args, log_failure: nil, use_context: true, use_namespace: true, output:

(1..attempts).to_a.each do |current_attempt|
logger.debug("Running command (attempt #{current_attempt}): #{cmd.join(' ')}")
out, err, st = Open3.capture3(*cmd)
env = { 'KUBECONFIG' => kubeconfig }
out, err, st = Open3.capture3(env, *cmd)

# https://github.com/Shopify/krane/issues/395
unless out.valid_encoding?
Expand Down Expand Up @@ -62,7 +63,8 @@ def run(*args, log_failure: nil, use_context: true, use_namespace: true, output:
else
logger.debug("Kubectl err: #{output_is_sensitive ? '<suppressed sensitive output>' : err}")
end
StatsD.client.increment('kubectl.error', 1, tags: { context: context, namespace: namespace, cmd: cmd[1] })
StatsD.client.increment('kubectl.error', 1, tags: { context: context, namespace: namespace, cmd: cmd[1],
max_attempt: attempts, current_attempt: current_attempt })

break unless retriable_err?(err, retry_whitelist) && current_attempt < attempts
sleep(retry_delay(current_attempt))
Expand All @@ -79,7 +81,7 @@ def retry_delay(attempt)
def version_info
@version_info ||=
begin
response, _, status = run("version", use_namespace: false, log_failure: true)
response, _, status = run("version", use_namespace: false, log_failure: true, attempts: 2)
raise KubectlError, "Could not retrieve kubectl version info" unless status.success?
extract_version_info_from_kubectl_response(response)
end
Expand Down
1 change: 1 addition & 0 deletions lib/krane/kubernetes_resource/persistent_volume_claim.rb
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ class StorageClass < KubernetesResource
attr_reader :name

def initialize(definition)
super(definition: definition, namespace: nil, context: nil, logger: nil)
@definition = definition
@name = definition.dig("metadata", "name").to_s
end
Expand Down
2 changes: 1 addition & 1 deletion lib/krane/resource_deployer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ def apply_all(resources, prune)
output_is_sensitive = resources.any?(&:sensitive_template_content?)
global_mode = resources.all?(&:global?)
out, err, st = kubectl.run(*command, log_failure: false, output_is_sensitive: output_is_sensitive,
use_namespace: !global_mode)
attempts: 2, use_namespace: !global_mode)

if st.success?
log_pruning(out) if prune
Expand Down
12 changes: 6 additions & 6 deletions lib/krane/restart_task.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,19 @@ def initialize(deployment_name, response)
HTTP_OK_RANGE = 200..299
ANNOTATION = "shipit.shopify.io/restart"

attr_reader :task_config

delegate :kubeclient_builder, to: :task_config

# Initializes the restart task
#
# @param context [String] Kubernetes context / cluster (*required*)
# @param namespace [String] Kubernetes namespace (*required*)
# @param logger [Object] Logger object (defaults to an instance of Krane::FormattedLogger)
# @param global_timeout [Integer] Timeout in seconds
def initialize(context:, namespace:, logger: nil, global_timeout: nil)
def initialize(context:, namespace:, logger: nil, global_timeout: nil, kubeconfig: nil)
@logger = logger || Krane::FormattedLogger.build(namespace, context)
@task_config = Krane::TaskConfig.new(context, namespace, @logger)
@task_config = Krane::TaskConfig.new(context, namespace, @logger, kubeconfig)
@context = context
@namespace = namespace
@global_timeout = global_timeout
Expand Down Expand Up @@ -220,9 +224,5 @@ def kubectl
def v1beta1_kubeclient
@v1beta1_kubeclient ||= kubeclient_builder.build_v1beta1_kubeclient(@context)
end

def kubeclient_builder
@kubeclient_builder ||= KubeclientBuilder.new
end
end
end
12 changes: 5 additions & 7 deletions lib/krane/runner_task.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,17 +16,19 @@ module Krane
class RunnerTask
class TaskTemplateMissingError < TaskConfigurationError; end

attr_reader :pod_name
attr_reader :pod_name, :task_config

delegate :kubeclient_builder, to: :task_config

# Initializes the runner task
#
# @param namespace [String] Kubernetes namespace (*required*)
# @param context [String] Kubernetes context / cluster (*required*)
# @param logger [Object] Logger object (defaults to an instance of Krane::FormattedLogger)
# @param global_timeout [Integer] Timeout in seconds
def initialize(namespace:, context:, logger: nil, global_timeout: nil)
def initialize(namespace:, context:, logger: nil, global_timeout: nil, kubeconfig: nil)
@logger = logger || Krane::FormattedLogger.build(namespace, context)
@task_config = Krane::TaskConfig.new(context, namespace, @logger)
@task_config = Krane::TaskConfig.new(context, namespace, @logger, kubeconfig)
@namespace = namespace
@context = context
@global_timeout = global_timeout
Expand Down Expand Up @@ -200,10 +202,6 @@ def kubeclient
@kubeclient ||= kubeclient_builder.build_v1_kubeclient(@context)
end

def kubeclient_builder
@kubeclient_builder ||= KubeclientBuilder.new
end

def statsd_tags(status)
%W(namespace:#{@namespace} context:#{@context} status:#{status})
end
Expand Down
9 changes: 7 additions & 2 deletions lib/krane/task_config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,13 @@

module Krane
class TaskConfig
attr_reader :context, :namespace, :logger
attr_reader :context, :namespace, :logger, :kubeconfig

def initialize(context, namespace, logger = nil)
def initialize(context, namespace, logger = nil, kubeconfig = nil)
@context = context
@namespace = namespace
@logger = logger || FormattedLogger.build(@namespace, @context)
@kubeconfig = kubeconfig || ENV['KUBECONFIG']
end

def global_kinds
Expand All @@ -18,5 +19,9 @@ def global_kinds
cluster_resource_discoverer.fetch_resources(namespaced: false).map { |g| g["kind"] }
end
end

def kubeclient_builder
@kubeclient_builder ||= KubeclientBuilder.new(kubeconfig: kubeconfig)
end
end
end
6 changes: 3 additions & 3 deletions lib/krane/task_config_validator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ def validate_context_exists_in_kubeconfig
end

_, err, st = @kubectl.run("config", "get-contexts", context, "-o", "name",
use_namespace: false, use_context: false, log_failure: false)
use_namespace: false, use_context: false, log_failure: false, attempts: 2)

unless st.success?
@errors << if err.match("error: context #{context} not found")
Expand All @@ -58,7 +58,7 @@ def validate_context_exists_in_kubeconfig

def validate_context_reachable
_, err, st = @kubectl.run("get", "namespaces", "-o", "name",
use_namespace: false, log_failure: false)
use_namespace: false, log_failure: false, attempts: 2)

unless st.success?
@errors << "Something went wrong connecting to #{context}. #{err} "
Expand All @@ -71,7 +71,7 @@ def validate_namespace_exists
end

_, err, st = @kubectl.run("get", "namespace", "-o", "name", namespace,
use_namespace: false, log_failure: false)
use_namespace: false, log_failure: false, attempts: 3)

unless st.success?
@errors << if err.match("Error from server [(]NotFound[)]: namespace")
Expand Down
2 changes: 1 addition & 1 deletion test/helpers/fixture_deploy_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ def deploy_raw_fixtures(set, wait: true, bindings: {}, subset: nil, render_erb:
def deploy_dirs_without_profiling(dirs, wait: true, prune: true, bindings: {},
sha: "k#{SecureRandom.hex(6)}", kubectl_instance: nil, global_timeout: nil, selector: nil,
protected_namespaces: nil, render_erb: false)
kubectl_instance ||= build_kubectl
kubectl_instance = build_kubectl

deploy = Krane::DeployTask.new(
namespace: @namespace,
Expand Down
2 changes: 2 additions & 0 deletions test/helpers/fixture_sets/cronjobs.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
# frozen_string_literal: true
# rubocop:disable Lint/MissingSuper

module FixtureSetAssertions
class CronJobs < FixtureSet
def initialize(namespace)
Expand Down
2 changes: 2 additions & 0 deletions test/helpers/fixture_sets/ejson_cloud.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
# frozen_string_literal: true
# rubocop:disable Lint/MissingSuper

module FixtureSetAssertions
class EjsonCloud < FixtureSet
def initialize(namespace)
Expand Down
2 changes: 2 additions & 0 deletions test/helpers/fixture_sets/hello_cloud.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
# frozen_string_literal: true
# rubocop:disable Lint/MissingSuper

module FixtureSetAssertions
class HelloCloud < FixtureSet
def initialize(namespace)
Expand Down
26 changes: 13 additions & 13 deletions test/helpers/kubeclient_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,54 +9,54 @@ module KubeclientHelper
TEST_CONTEXT ||= "minikube"

def kubeclient
@kubeclient ||= kubeclient_builder.build_v1_kubeclient(TEST_CONTEXT)
kubeclient_builder.build_v1_kubeclient(TEST_CONTEXT)
end

def v1beta1_kubeclient
@v1beta1_kubeclient ||= kubeclient_builder.build_v1beta1_kubeclient(TEST_CONTEXT)
kubeclient_builder.build_v1beta1_kubeclient(TEST_CONTEXT)
end

def policy_v1beta1_kubeclient
@policy_v1beta1_kubeclient ||= kubeclient_builder.build_policy_v1beta1_kubeclient(TEST_CONTEXT)
kubeclient_builder.build_policy_v1beta1_kubeclient(TEST_CONTEXT)
end

def apps_v1_kubeclient
@apps_v1_kubeclient ||= kubeclient_builder.build_apps_v1_kubeclient(TEST_CONTEXT)
kubeclient_builder.build_apps_v1_kubeclient(TEST_CONTEXT)
end

def batch_v1beta1_kubeclient
@batch_v1beta1_kubeclient ||= kubeclient_builder.build_batch_v1beta1_kubeclient(TEST_CONTEXT)
kubeclient_builder.build_batch_v1beta1_kubeclient(TEST_CONTEXT)
end

def batch_v1_kubeclient
@batch_v1_kubeclient ||= kubeclient_builder.build_batch_v1_kubeclient(TEST_CONTEXT)
kubeclient_builder.build_batch_v1_kubeclient(TEST_CONTEXT)
end

def apiextensions_v1beta1_kubeclient
@apiextensions_v1beta1_kubeclient ||= kubeclient_builder.build_apiextensions_v1beta1_kubeclient(TEST_CONTEXT)
kubeclient_builder.build_apiextensions_v1beta1_kubeclient(TEST_CONTEXT)
end

def autoscaling_v1_kubeclient
@autoscaling_v1_kubeclient ||= kubeclient_builder.build_autoscaling_v1_kubeclient(TEST_CONTEXT)
kubeclient_builder.build_autoscaling_v1_kubeclient(TEST_CONTEXT)
end

def rbac_v1_kubeclient
@rbac_v1_kubeclient ||= kubeclient_builder.build_rbac_v1_kubeclient(TEST_CONTEXT)
kubeclient_builder.build_rbac_v1_kubeclient(TEST_CONTEXT)
end

def networking_v1_kubeclient
@networking_v1_kubeclient ||= kubeclient_builder.build_networking_v1_kubeclient(TEST_CONTEXT)
kubeclient_builder.build_networking_v1_kubeclient(TEST_CONTEXT)
end

def storage_v1_kubeclient
@storage_v1_kubeclient ||= kubeclient_builder.build_storage_v1_kubeclient(TEST_CONTEXT)
kubeclient_builder.build_storage_v1_kubeclient(TEST_CONTEXT)
end

def scheduling_v1beta1_kubeclient
@scheduling_v1beta1_kubeclient ||= kubeclient_builder.build_scheduling_v1beta1_kubeclient(TEST_CONTEXT)
kubeclient_builder.build_scheduling_v1beta1_kubeclient(TEST_CONTEXT)
end

def kubeclient_builder
@kubeclient_builder ||= Krane::KubeclientBuilder.new
Krane::KubeclientBuilder.new
end
end
2 changes: 1 addition & 1 deletion test/helpers/mock_resource.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ def after_sync
def type
"MockResource"
end
alias_method :kubectl_resource_type, :type
alias_method(:kubectl_resource_type, :type)

def group
"core"
Expand Down
Loading