Skip to content

Commit

Permalink
refactor to avoid passing template_dirs all around
Browse files Browse the repository at this point in the history
hacky fix for render task, add test for partial naming collision

test result, not forced failure

deploy_dir -> deploy_dirs
  • Loading branch information
timothysmith0609 committed Jul 24, 2019
1 parent 79d77dc commit a60dbcf
Show file tree
Hide file tree
Showing 8 changed files with 49 additions and 18 deletions.
13 changes: 6 additions & 7 deletions lib/kubernetes-deploy/deploy_task.rb
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,8 @@ def initialize(namespace:, context:, current_sha:, template_dirs:, logger:, kube
@logger = logger
@kubectl = kubectl_instance
@max_watch_seconds = max_watch_seconds
@renderers = @template_dirs.map do |template_dir|
KubernetesDeploy::Renderer.new(
@renderers = Hash.new do |hash, template_dir|
hash[template_dir] = KubernetesDeploy::Renderer.new(
current_sha: @current_sha,
template_dir: template_dir,
logger: @logger,
Expand Down Expand Up @@ -279,8 +279,8 @@ def discover_resources
crds = cluster_resource_discoverer.crds.group_by(&:kind)
@logger.info("Discovering templates:")

@template_dirs.each do |template_dir|
TemplateDiscovery.new(template_dir).templates.each do |filename|
TemplateDiscovery.new(@template_dirs).templates.each do |template_dir, filenames|
filenames.each do |filename|
split_templates(template_dir, filename) do |r_def|
crd = crds[r_def["kind"]]&.first
r = KubernetesResource.build(namespace: @namespace, context: @context, logger: @logger, definition: r_def,
Expand All @@ -290,6 +290,7 @@ def discover_resources
end
end
end

secrets_from_ejson.each do |secret|
resources << secret
@logger.info(" - #{secret.id} (from ejson)")
Expand All @@ -304,9 +305,7 @@ def discover_resources

def split_templates(template_dir, filename)
file_content = File.read(File.join(template_dir, filename))
rendered_content = @renderers.find do |renderer|
renderer.template_dir == template_dir
end.render_template(filename, file_content)
rendered_content = @renderers[template_dir].render_template(filename, file_content)
YAML.load_stream(rendered_content, "<rendered> #{filename}") do |doc|
next if doc.blank?
unless doc.is_a?(Hash)
Expand Down
2 changes: 1 addition & 1 deletion lib/kubernetes-deploy/render_task.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ def run!(stream, only_filenames = [])
@logger.phase_heading("Initializing render task")

filenames = if only_filenames.empty?
TemplateDiscovery.new(@template_dir).templates
TemplateDiscovery.new([@template_dir]).templates[@template_dir]
else
only_filenames
end
Expand Down
1 change: 0 additions & 1 deletion lib/kubernetes-deploy/renderer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ def initialize(msg, parents: [], content: nil, filename:)
end
end
class PartialNotFound < InvalidTemplateError; end
attr_reader :template_dir

def initialize(current_sha:, template_dir:, logger:, bindings: {})
@current_sha = current_sha
Expand Down
10 changes: 6 additions & 4 deletions lib/kubernetes-deploy/template_discovery.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,15 @@

module KubernetesDeploy
class TemplateDiscovery
def initialize(template_dir)
@template_dir = template_dir
def initialize(template_dirs)
@template_dirs = template_dirs
end

def templates
Dir.foreach(@template_dir).select do |filename|
filename.end_with?(".yml.erb", ".yml", ".yaml", ".yaml.erb")
@template_dirs.each_with_object({}) do |template_dir, hash|
hash[template_dir] = Dir.foreach(template_dir).select do |filename|
filename.end_with?(".yml.erb", ".yml", ".yaml", ".yaml.erb")
end
end
end
end
Expand Down
4 changes: 4 additions & 0 deletions test/fixtures/test-partials2/deployment.yml.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
apiVersion: extensions/v1beta1
kind: Deployment
<<: <%= partial 'pod' %> # Use same partial name as `test-partials` dir to ensure no naming collision
15 changes: 15 additions & 0 deletions test/fixtures/test-partials2/partials/pod.yaml.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
metadata:
name: web-from-partial
spec:
replicas: 1
template:
metadata:
labels:
name: web-from-partial
app: test-partials
spec:
containers:
- name: partial-pod
image: busybox
imagePullPolicy: IfNotPresent
command: ["sleep", "8000"]
10 changes: 5 additions & 5 deletions test/helpers/fixture_deploy_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ def deploy_fixtures(set, subset: nil, **args) # extra args are passed through to
success = false
Dir.mktmpdir("fixture_dir") do |target_dir|
write_fixtures_to_dir(fixtures, target_dir)
success = deploy_dir(target_dir, args)
success = deploy_dirs(target_dir, args)
end
success
end
Expand All @@ -52,10 +52,10 @@ def deploy_raw_fixtures(set, wait: true, bindings: {}, subset: nil)
subset.each do |file|
FileUtils.copy_entry(File.join(fixture_path(set), file), File.join(target_dir, file))
end
success = deploy_dir(target_dir, wait: wait, bindings: bindings)
success = deploy_dirs(target_dir, wait: wait, bindings: bindings)
end
else
success = deploy_dir(fixture_path(set), wait: wait, bindings: bindings)
success = deploy_dirs(fixture_path(set), wait: wait, bindings: bindings)
end
success
end
Expand All @@ -82,10 +82,10 @@ def deploy_dirs_without_profiling(dirs, wait: true, allow_protected_ns: false, p
)
end

# Deploys all fixtures in the given directory via KubernetesDeploy::DeployTask
# Deploys all fixtures in the given directories via KubernetesDeploy::DeployTask
# Exposed for direct use only when deploy_fixtures cannot be used because the template cannot be loaded pre-deploy,
# for example because it contains an intentional syntax error
def deploy_dir(*dirs, **args)
def deploy_dirs(*dirs, **args)
if ENV["PROFILE"]
deploy_result = nil
result = RubyProf.profile { deploy_result = deploy_dirs_without_profiling(dirs, args) }
Expand Down
12 changes: 12 additions & 0 deletions test/integration/kubernetes_deploy_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -802,6 +802,18 @@ def test_deploy_successful_with_partial_availability
assert_equal(1, new_ready_pods.length, "Expected exactly one new pod to be ready, saw #{new_ready_pods.length}")
end

def test_deploy_successful_with_multiple_template_dirs
result = deploy_dirs(fixture_path("test-partials"), fixture_path("cronjobs"),
bindings: { 'supports_partials' => 'yep' })
assert_deploy_success(result)
end

def test_deploy_successful_with_multiple_template_dirs_multiple_partials
result = deploy_dirs(fixture_path("test-partials"), fixture_path("test-partials2"),
bindings: { 'supports_partials' => 'yep' })
assert_deploy_success(result)
end

def test_deploy_aborts_immediately_if_metadata_name_missing
result = deploy_fixtures("hello-cloud", subset: ["configmap-data.yml"]) do |fixtures|
definition = fixtures["configmap-data.yml"]["ConfigMap"].first
Expand Down

0 comments on commit a60dbcf

Please sign in to comment.