Skip to content

Commit

Permalink
Merge pull request #533 from Shopify/consistent-validations
Browse files Browse the repository at this point in the history
First pass at shared validation
  • Loading branch information
dturn committed Aug 30, 2019
2 parents 410812f + 641d7a6 commit 29e29a5
Show file tree
Hide file tree
Showing 14 changed files with 229 additions and 37 deletions.
2 changes: 2 additions & 0 deletions lib/kubernetes-deploy/common.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
require 'kubernetes-deploy/errors'
require 'kubernetes-deploy/formatted_logger'
require 'kubernetes-deploy/statsd'
require 'kubernetes-deploy/task_config'
require 'kubernetes-deploy/task_config_validator'

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

def initialize(namespace:, context:, current_sha:, template_dir:, logger: nil, kubectl_instance: nil, bindings: {},
max_watch_seconds: nil, selector: nil)
@logger = logger || KubernetesDeploy::FormattedLogger.build(namespace, context)
@task_config = KubernetesDeploy::TaskConfig.new(context, namespace, @logger)
@namespace = namespace
@namespace_tags = []
@context = context
@current_sha = current_sha
@template_dir = File.expand_path(template_dir)
@logger = logger || KubernetesDeploy::FormattedLogger.build(namespace, context)
@kubectl = kubectl_instance
@max_watch_seconds = max_watch_seconds
@renderer = KubernetesDeploy::Renderer.new(
Expand Down Expand Up @@ -551,9 +552,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, kubectl, kubeclient_builder, 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
4 changes: 2 additions & 2 deletions lib/kubernetes-deploy/kubeclient_builder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -106,11 +106,11 @@ def build_storage_v1_kubeclient(context)
def validate_config_files
errors = []
if @kubeconfig_files.empty?
errors << "Kube config file name(s) not set in $KUBECONFIG"
errors << "Kubeconfig file name(s) not set in $KUBECONFIG"
else
@kubeconfig_files.each do |f|
# If any files in the list are not valid, we can't be sure the merged context list is what the user intended
errors << "Kube config not found at #{f}" unless File.file?(f)
errors << "Kubeconfig not found at #{f}" unless File.file?(f)
end
end
errors
Expand Down
25 changes: 13 additions & 12 deletions lib/kubernetes-deploy/restart_task.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,10 @@ def initialize(deployment_name, response)
ANNOTATION = "shipit.shopify.io/restart"

def initialize(context:, namespace:, logger: nil, max_watch_seconds: nil)
@logger = logger || KubernetesDeploy::FormattedLogger.build(namespace, context)
@task_config = KubernetesDeploy::TaskConfig.new(context, namespace, @logger)
@context = context
@namespace = namespace
@logger = logger || KubernetesDeploy::FormattedLogger.build(namespace, context)
@max_watch_seconds = max_watch_seconds
end

Expand All @@ -40,11 +41,9 @@ def perform!(deployments_names = nil, selector: nil)
@logger.reset

@logger.phase_heading("Initializing restart")
verify_namespace
verify_config!
deployments = identify_target_deployments(deployments_names, selector: selector)
if kubectl.server_version < Gem::Version.new(MIN_KUBE_VERSION)
@logger.warn(KubernetesDeploy::Errors.server_version_warning(kubectl.server_version))
end

@logger.phase_heading("Triggering restart by touching ENV[RESTARTED_AT]")
patch_kubeclient_deployments(deployments)

Expand Down Expand Up @@ -120,13 +119,6 @@ def build_watchables(kubeclient_resources, started)
end
end

def verify_namespace
kubeclient.get_namespace(@namespace)
@logger.info("Namespace #{@namespace} found in context #{@context}")
rescue Kubeclient::ResourceNotFoundError
raise NamespaceNotFoundError.new(@namespace, @context)
end

def patch_deployment_with_restart(record)
v1beta1_kubeclient.patch_deployment(
record.metadata.name,
Expand Down Expand Up @@ -176,6 +168,15 @@ def build_patch_payload(deployment)
}
end

def verify_config!
task_config_validator = TaskConfigValidator.new(@task_config, kubectl, kubeclient_builder)
unless task_config_validator.valid?
@logger.summary.add_action("Configuration invalid")
@logger.summary.add_paragraph(task_config_validator.errors.map { |err| "- #{err}" }.join("\n"))
raise KubernetesDeploy::TaskConfigurationError
end
end

def kubeclient
@kubeclient ||= kubeclient_builder.build_v1_kubeclient(@context)
end
Expand Down
5 changes: 2 additions & 3 deletions lib/kubernetes-deploy/runner_task.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ class TaskTemplateMissingError < TaskConfigurationError; end

def initialize(namespace:, context:, logger: nil, max_watch_seconds: nil)
@logger = logger || KubernetesDeploy::FormattedLogger.build(namespace, context)
@task_config = KubernetesDeploy::TaskConfig.new(context, namespace, @logger)
@namespace = namespace
@context = context
@max_watch_seconds = max_watch_seconds
Expand Down Expand Up @@ -140,9 +141,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, kubectl, kubeclient_builder, only: [:validate_server_version]).valid?
end

def get_template(template_name)
Expand Down
16 changes: 16 additions & 0 deletions lib/kubernetes-deploy/task_config.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# frozen_string_literal: true
module KubernetesDeploy
class TaskConfig
attr_reader :context, :namespace

def initialize(context, namespace, logger = nil)
@context = context
@namespace = namespace
@logger = logger
end

def logger
@logger ||= KubernetesDeploy::FormattedLogger.build(@namespace, @context)
end
end
end
96 changes: 96 additions & 0 deletions lib/kubernetes-deploy/task_config_validator.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
# frozen_string_literal: true
module KubernetesDeploy
class TaskConfigValidator
DEFAULT_VALIDATIONS = %i(
validate_kubeconfig
validate_context_exists_in_kubeconfig
validate_context_reachable
validate_server_version
validate_namespace_exists
).freeze

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

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

def valid?
@errors = []
@validations.each do |validator_name|
break if @errors.present?
send(validator_name)
end
@errors.empty?
end

def errors
valid?
@errors
end

private

def validate_kubeconfig
@errors += @kubeclient_builder.validate_config_files
end

def validate_context_exists_in_kubeconfig
unless context.present?
return @errors << "Context can not be blank"
end

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

unless st.success?
@errors << if err.match("error: context #{context} not found")
"Context #{context} missing from your kubeconfig file(s)"
else
"Something went wrong. #{err} "
end
end
end

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

unless st.success?
@errors << "Something went wrong connecting to #{context}. #{err} "
end
end

def validate_namespace_exists
unless namespace.present?
return @errors << "Namespace can not be blank"
end

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

unless st.success?
@errors << if err.match("Error from server [(]NotFound[)]: namespace")
"Could not find Namespace: #{namespace} in Context: #{context}"
else
"Could not connect to kubernetes cluster. #{err}"
end
end
end

def validate_server_version
if @kubectl.server_version < Gem::Version.new(MIN_KUBE_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
4 changes: 2 additions & 2 deletions test/integration-serial/serial_deploy_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ def test_multiple_configuration_files
assert_logs_match_all([
'Result: FAILURE',
'Configuration invalid',
"Kube config not found at #{config_file}",
"Kubeconfig not found at #{config_file}",
], in_order: true)
reset_logger

Expand All @@ -85,7 +85,7 @@ def test_multiple_configuration_files
assert_logs_match_all([
'Result: FAILURE',
'Configuration invalid',
"Kube config file name(s) not set in $KUBECONFIG",
"Kubeconfig file name(s) not set in $KUBECONFIG",
], in_order: true)
reset_logger

Expand Down
4 changes: 2 additions & 2 deletions test/integration/restart_task_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ def test_restart_not_existing_context
assert_restart_failure(restart.perform(%w(web)))
assert_logs_match_all([
"Result: FAILURE",
"`walrus` context must be configured in your kubeconfig file(s)",
/- Context walrus missing from your kubeconfig file\(s\)/,
],
in_order: true)
end
Expand All @@ -188,7 +188,7 @@ def test_restart_not_existing_namespace
assert_restart_failure(restart.perform(%w(web)))
assert_logs_match_all([
"Result: FAILURE",
"Namespace `walrus` not found in context `#{TEST_CONTEXT}`",
"- Could not find Namespace: walrus in Context: #{KubeclientHelper::TEST_CONTEXT}",
],
in_order: true)
end
Expand Down
60 changes: 60 additions & 0 deletions test/integration/task_config_validator_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
# frozen_string_literal: true
require 'integration_test_helper'

class TaskConfigValidatorTest < KubernetesDeploy::IntegrationTest
def test_valid_configuration
assert_predicate(validator(context: KubeclientHelper::TEST_CONTEXT, namespace: 'default'), :valid?)
end

def test_only_is_respected
assert_predicate(validator(only: []), :valid?)
end

def test_invalid_kubeconfig
bad_file = "/IM_NOT_A_REAL_FILE.yml"
builder = KubernetesDeploy::KubeclientBuilder.new(kubeconfig: bad_file)
assert_match("Kubeconfig not found at #{bad_file}",
validator(kubeclient_builder: builder, only: [:validate_kubeconfig]).errors.join("\n"))
end

def test_context_does_not_exists_in_kubeconfig
fake_context = "fake-context"
assert_match(/Context #{fake_context} missing from your kubeconfig file/,
validator(context: fake_context).errors.join("\n"))
end

def test_context_not_reachable
fake_context = "fake-context"
assert_match(/Something went wrong connecting to #{fake_context}/,
validator(context: fake_context, only: [:validate_context_reachable]).errors.join("\n"))
end

def test_namespace_does_not_exists
assert_match(/Could not find Namespace: test-namespace in Context: #{KubeclientHelper::TEST_CONTEXT}/,
validator(context: KubeclientHelper::TEST_CONTEXT).errors.join("\n"))
end

def test_invalid_server_version
old_min_version = KubernetesDeploy::MIN_KUBE_VERSION
new_min_version = "99999"
KubernetesDeploy.const_set(:MIN_KUBE_VERSION, new_min_version)
validator(context: KubeclientHelper::TEST_CONTEXT, namespace: 'default', logger: @logger).valid?
assert_logs_match_all([
"Minimum cluster version requirement of #{new_min_version} not met.",
])
ensure
KubernetesDeploy.const_set(:MIN_KUBE_VERSION, old_min_version)
end

private

def validator(context: nil, namespace: nil, logger: nil, kubeclient_builder: nil, only: nil)
context ||= "test-context"
namespace ||= "test-namespace"
config = task_config(context: context, namespace: namespace, logger: logger)
kubectl = KubernetesDeploy::Kubectl.new(namespace: config.namespace,
context: config.context, logger: config.logger, log_failure_by_default: true)
kubeclient_builder ||= KubernetesDeploy::KubeclientBuilder.new
KubernetesDeploy::TaskConfigValidator.new(config, kubectl, kubeclient_builder, only: only)
end
end
4 changes: 4 additions & 0 deletions test/test_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,10 @@ def mock_output_stream
end
end

def task_config(context: KubeclientHelper::TEST_CONTEXT, namespace: @namespace, logger: @logger)
KubernetesDeploy::TaskConfig.new(context, namespace, logger)
end

private

def log_to_real_fds?
Expand Down
8 changes: 4 additions & 4 deletions test/unit/kubernetes-deploy/kubeclient_builder_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,14 @@ def test_config_validation_missing_file
builder = KubernetesDeploy::KubeclientBuilder.new(kubeconfig: config_file)
errors = builder.validate_config_files
assert_equal(1, errors.length)
assert_equal(errors.first, "Kube config not found at #{config_file}")
assert_equal(errors.first, "Kubeconfig not found at #{config_file}")
end

def test_build_runs_config_validation
config_file = File.join(__dir__, '../../fixtures/kube-config/unknown_config.yml')
kubeclient_builder = KubernetesDeploy::KubeclientBuilder.new(kubeconfig: config_file)

expected_err = /Kube config not found at .*unknown_config.yml/
expected_err = /Kubeconfig not found at .*unknown_config.yml/
assert_raises_message(KubernetesDeploy::TaskConfigurationError, expected_err) do
kubeclient_builder.build_v1_kubeclient('test-context')
end
Expand All @@ -30,12 +30,12 @@ def test_no_config_files_specified
builder = KubernetesDeploy::KubeclientBuilder.new(kubeconfig: " : ")
errors = builder.validate_config_files
assert_equal(1, errors.length)
assert_equal(errors.first, "Kube config file name(s) not set in $KUBECONFIG")
assert_equal(errors.first, "Kubeconfig file name(s) not set in $KUBECONFIG")

builder = KubernetesDeploy::KubeclientBuilder.new(kubeconfig: "")
errors = builder.validate_config_files
assert_equal(1, errors.length)
assert_equal(errors.first, "Kube config file name(s) not set in $KUBECONFIG")
assert_equal(errors.first, "Kubeconfig file name(s) not set in $KUBECONFIG")
end

def test_multiple_valid_configuration_files
Expand Down
Loading

0 comments on commit 29e29a5

Please sign in to comment.