Skip to content

Commit

Permalink
PR feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
dturn committed Aug 26, 2019
1 parent 6477c39 commit 6edce21
Show file tree
Hide file tree
Showing 7 changed files with 22 additions and 39 deletions.
2 changes: 1 addition & 1 deletion lib/kubernetes-deploy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
require 'kubernetes-deploy/resource_cache'
require 'kubernetes-deploy/label_selector'
require 'kubernetes-deploy/task_config'
require 'kubernetes-deploy/validator'
require 'kubernetes-deploy/task_config_validator'

module KubernetesDeploy
MIN_KUBE_VERSION = '1.10.0'
Expand Down
5 changes: 2 additions & 3 deletions lib/kubernetes-deploy/deploy_task.rb
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ def server_version

def initialize(namespace:, context:, current_sha:, template_dir:, logger:, kubectl_instance: nil, bindings: {},
max_watch_seconds: nil, selector: nil)
@task_config = KubernetesDeploy::TaskConfig.new(context, namespace, logger)
@namespace = namespace
@namespace_tags = []
@context = context
Expand Down Expand Up @@ -550,9 +551,7 @@ def confirm_cluster_reachable
end
end
raise FatalDeploymentError, "Failed to reach server for #{@context}" unless success
if kubectl.server_version < Gem::Version.new(MIN_KUBE_VERSION)
@logger.warn(KubernetesDeploy::Errors.server_version_warning(server_version))
end
TaskConfigValidator.new(@task_config, only: [:validate_server_version]).valid?
end

def confirm_namespace_exists
Expand Down
8 changes: 0 additions & 8 deletions lib/kubernetes-deploy/errors.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,4 @@ def initialize
"kubernetes-deploy will not continue since it is extremely unlikely that this secret should be pruned.")
end
end

module Errors
extend self
def server_version_warning(server_version)
"Minimum cluster version requirement of #{MIN_KUBE_VERSION} not met. "\
"Using #{server_version} could result in unexpected behavior as it is no longer tested against"
end
end
end
10 changes: 5 additions & 5 deletions lib/kubernetes-deploy/restart_task.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,10 @@ def perform!(deployments_names = nil, selector: nil)
@logger.reset

@logger.phase_heading("Initializing restart")
errors = Validator.new(@task_config).errors
unless errors.empty?
task_config_validator = TaskConfigValidator.new(@task_config)
unless task_config_validator.valid?
@logger.summary.add_action("Configuration invalid")
@logger.summary.add_paragraph(errors.map { |err| "- #{err}" }.join("\n"))
@logger.summary.add_paragraph(task_config_validator.errors.map { |err| "- #{err}" }.join("\n"))
raise KubernetesDeploy::TaskConfigurationError
end

Expand Down Expand Up @@ -175,15 +175,15 @@ def kubeclient
end

def kubectl
@task_config.kubectl
@kubectl ||= Kubectl.new(namespace: @namespace, context: @context, logger: @logger, log_failure_by_default: true)
end

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

def kubeclient_builder
@task_config.kubeclient_builder
@kubeclient_builder ||= KubeclientBuilder.new
end
end
end
5 changes: 2 additions & 3 deletions lib/kubernetes-deploy/runner_task.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ class TaskTemplateMissingError < TaskConfigurationError; end
attr_reader :pod_name

def initialize(namespace:, context:, logger:, max_watch_seconds: nil)
@task_config = KubernetesDeploy::TaskConfig.new(context, namespace, logger)
@logger = logger
@namespace = namespace
@context = context
Expand Down Expand Up @@ -135,9 +136,7 @@ def validate_configuration(task_template, args)
raise TaskConfigurationError, "Configuration invalid: #{errors.join(', ')}"
end

if kubectl.server_version < Gem::Version.new(MIN_KUBE_VERSION)
@logger.warn(KubernetesDeploy::Errors.server_version_warning(kubectl.server_version))
end
TaskConfigValidator.new(@task_config, only: [:validate_server_version]).valid?
end

def get_template(template_name)
Expand Down
13 changes: 0 additions & 13 deletions lib/kubernetes-deploy/task_config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,18 +12,5 @@ def initialize(context, namespace, logger = nil)
def logger
@logger ||= KubernetesDeploy::FormattedLogger.build(@namespace, @context)
end

def kubectl(log_failure_by_default: true)
@kubectl ||= Kubectl.new(
namespace: @namespace,
context: @context,
logger: logger,
log_failure_by_default: log_failure_by_default
)
end

def kubeclient_builder
@kubeclient ||= KubeclientBuilder.new
end
end
end
Original file line number Diff line number Diff line change
@@ -1,25 +1,26 @@
# frozen_string_literal: true
module KubernetesDeploy
class Validator
class TaskConfigValidator
DEFAULT_VALIDATIONS = %i(
validate_kubeconfig
validate_context_exists
validate_namespace_exists
validate_server_version
).freeze

delegate :context, :namespace, :logger, :kubectl, :kubeclient_builder, to: :@task_config
delegate :context, :namespace, :logger, to: :@task_config

def initialize(task_config)
def initialize(task_config, only: nil)
@task_config = task_config
@errors = nil
@validations = only || DEFAULT_VALIDATIONS
end

def valid?
@errors = []
DEFAULT_VALIDATIONS.each do |validator_name|
send(validator_name)
@validations.each do |validator_name|
break if @errors.present?
send(validator_name)
end
@errors.empty?
end
Expand Down Expand Up @@ -79,8 +80,13 @@ def validate_namespace_exists

def validate_server_version
if kubectl.server_version < Gem::Version.new(MIN_KUBE_VERSION)
logger.warn(KubernetesDeploy::Errors.server_version_warning(kubectl.server_version))
logger.warn(server_version_warning(kubectl.server_version))
end
end

def server_version_warning(server_version)
"Minimum cluster version requirement of #{MIN_KUBE_VERSION} not met. "\
"Using #{server_version} could result in unexpected behavior as it is no longer tested against"
end
end
end

0 comments on commit 6edce21

Please sign in to comment.