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

Add some helper environment variable to both main and init container #252

Merged
merged 2 commits into from
Mar 19, 2021
Merged
Show file tree
Hide file tree
Changes from all 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
30 changes: 26 additions & 4 deletions lib/ood_core/job/adapters/kubernetes/batch.rb
Original file line number Diff line number Diff line change
Expand Up @@ -153,26 +153,48 @@ def k8s_username
username_prefix.nil? ? username : "#{username_prefix}-#{username}"
end

def user
@user ||= Etc.getpwnam(username)
end

def home_dir
user.dir
end

def run_as_user
Etc.getpwnam(username).uid
user.uid
end

def run_as_group
Etc.getpwnam(username).gid
user.gid
end

def fs_group
run_as_group
end

def group
Etc.getgrgid(run_as_group).name
end

def default_env
{
USER: username,
UID: run_as_user,
HOME: home_dir,
GROUP: group,
GID: run_as_group,
}
end

# helper to template resource yml you're going to submit and
# create an id.
def generate_id_yml(script)
native_data = script.native
container = helper.container_from_native(native_data[:container])
container = helper.container_from_native(native_data[:container], default_env)
id = generate_id(container.name)
configmap = helper.configmap_from_native(native_data, id)
init_containers = helper.init_ctrs_from_native(native_data[:init_containers])
init_containers = helper.init_ctrs_from_native(native_data[:init_containers], container.env)
spec = OodCore::Job::Adapters::Kubernetes::Resources::PodSpec.new(container, init_containers: init_containers)
all_mounts = native_data[:mounts].nil? ? mounts : mounts + native_data[:mounts]

Expand Down
15 changes: 10 additions & 5 deletions lib/ood_core/job/adapters/kubernetes/helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,18 +38,21 @@ def info_from_json(pod_json: nil, service_json: nil, secret_json: nil, ns_prefix
#
# @param container [#to_h]
# the input container hash
# @param default_env [#to_h]
# Default env to merge with defined env
# @return [OodCore::Job::Adapters::Kubernetes::Resources::Container]
def container_from_native(container)
def container_from_native(container, default_env)
env = container.fetch(:env, {}).to_h.symbolize_keys
OodCore::Job::Adapters::Kubernetes::Resources::Container.new(
container[:name],
container[:image],
command: parse_command(container[:command]),
port: container[:port],
env: container.fetch(:env, []),
env: default_env.merge(env),
memory: container[:memory],
cpu: container[:cpu],
working_dir: container[:working_dir],
restart_policy: container[:restart_policy]
restart_policy: container[:restart_policy],
)
end

Expand Down Expand Up @@ -93,13 +96,15 @@ def configmap_from_native(native, id)
# @param native_data [#to_h]
# the native data to parse. Expected key init_ctrs and for that
# key to be an array of hashes.
# @param default_env [#to_h]
# Default env to merge with defined env
# @return [Array<OodCore::Job::Adapters::Kubernetes::Resources::Container>]
# the array of init containers
def init_ctrs_from_native(ctrs)
def init_ctrs_from_native(ctrs, default_env)
init_ctrs = []

ctrs&.each do |ctr_raw|
ctr = container_from_native(ctr_raw)
ctr = container_from_native(ctr_raw, default_env)
init_ctrs.push(ctr)
end

Expand Down
5 changes: 2 additions & 3 deletions lib/ood_core/job/adapters/kubernetes/resources.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ class Container
:restart_policy, :supplemental_groups

def initialize(
name, image, command: [], port: nil, env: [], memory: "4Gi", cpu: "1",
name, image, command: [], port: nil, env: {}, memory: "4Gi", cpu: "1",
working_dir: "", restart_policy: "Never", supplemental_groups: []
)
raise ArgumentError, "containers need valid names and images" unless name && image
Expand All @@ -24,7 +24,7 @@ def initialize(
@image = image
@command = command.nil? ? [] : command
@port = port&.to_i
@env = env.nil? ? [] : env
@env = env.nil? ? {} : env
@memory = memory.nil? ? "4Gi" : memory
@cpu = cpu.nil? ? "1" : cpu
@working_dir = working_dir.nil? ? "" : working_dir
Expand All @@ -44,7 +44,6 @@ def ==(other)
restart_policy == other.restart_policy &&
supplemental_groups == other.supplemental_groups
end

end

class PodSpec
Expand Down
21 changes: 16 additions & 5 deletions lib/ood_core/job/adapters/kubernetes/templates/pod.yml.erb
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,15 @@ spec:
<%- unless spec.container.working_dir.empty? -%>
workingDir: "<%= spec.container.working_dir %>"
<%- end -%>
<%- unless spec.container.env.empty? -%>
env:
<%- spec.container.env.each do |env| -%>
- name: <%= env[:name] %>
value: "<%= env[:value] %>"
- name: POD_NAME
valueFrom:
fieldRef:
fieldPath: metadata.name
<%- spec.container.env.each_pair do |name, value| -%>
- name: <%= name %>
value: "<%= value %>"
<%- end # for each env -%>
<%- end # unless env is nil -%>
<%- unless spec.container.command.empty? -%>
command:
<%- spec.container.command.each do |cmd| -%>
Expand Down Expand Up @@ -83,6 +85,15 @@ spec:
<%- spec.init_containers.each do |ctr| -%>
- name: "<%= ctr.name %>"
image: "<%= ctr.image %>"
env:
- name: POD_NAME
valueFrom:
fieldRef:
fieldPath: metadata.name
<%- ctr.env.each_pair do |name, value| -%>
- name: <%= name %>
value: "<%= value %>"
<%- end # for each env -%>
command:
<%- ctr.command.each do |cmd| -%>
- "<%= cmd %>"
Expand Down
29 changes: 29 additions & 0 deletions spec/fixtures/output/k8s/pod_yml_from_all_configs.yml
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,20 @@ spec:
imagePullPolicy: IfNotPresent
workingDir: "/my/home"
env:
- name: POD_NAME
valueFrom:
fieldRef:
fieldPath: metadata.name
- name: USER
value: "testuser"
- name: UID
value: "1001"
- name: HOME
value: "/my/home"
- name: GROUP
value: "testgroup"
- name: GID
value: "1002"
- name: PATH
value: "/usr/bin:/usr/local/bin"
command:
Expand Down Expand Up @@ -60,6 +72,23 @@ spec:
initContainers:
- name: "init-1"
image: "busybox:latest"
env:
- name: POD_NAME
valueFrom:
fieldRef:
fieldPath: metadata.name
- name: USER
value: "testuser"
- name: UID
value: "1001"
- name: HOME
value: "/my/home"
- name: GROUP
value: "testgroup"
- name: GID
value: "1002"
- name: PATH
value: "/usr/bin:/usr/local/bin"
command:
- "/bin/ls"
- "-lrt"
Expand Down
29 changes: 29 additions & 0 deletions spec/fixtures/output/k8s/pod_yml_from_defaults.yml
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,20 @@ spec:
imagePullPolicy: IfNotPresent
workingDir: "/my/home"
env:
- name: POD_NAME
valueFrom:
fieldRef:
fieldPath: metadata.name
- name: USER
value: "testuser"
- name: UID
value: "1001"
- name: HOME
value: "/my/home"
- name: GROUP
value: "testgroup"
- name: GID
value: "1002"
- name: PATH
value: "/usr/bin:/usr/local/bin"
command:
Expand Down Expand Up @@ -56,6 +68,23 @@ spec:
initContainers:
- name: "init-1"
image: "busybox:latest"
env:
- name: POD_NAME
valueFrom:
fieldRef:
fieldPath: metadata.name
- name: USER
value: "testuser"
- name: UID
value: "1001"
- name: HOME
value: "/my/home"
- name: GROUP
value: "testgroup"
- name: GID
value: "1002"
- name: PATH
value: "/usr/bin:/usr/local/bin"
command:
- "/bin/ls"
- "-lrt"
Expand Down
31 changes: 30 additions & 1 deletion spec/fixtures/output/k8s/pod_yml_no_mounts.yml
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,20 @@ spec:
imagePullPolicy: IfNotPresent
workingDir: "/my/home"
env:
- name: POD_NAME
valueFrom:
fieldRef:
fieldPath: metadata.name
- name: USER
value: "testuser"
- name: UID
value: "1001"
- name: HOME
value: "/my/home"
value: "/home/testuser"
- name: GROUP
value: "testgroup"
- name: GID
value: "1002"
- name: PATH
value: "/usr/bin:/usr/local/bin"
command:
Expand All @@ -54,6 +66,23 @@ spec:
initContainers:
- name: "init-1"
image: "busybox:latest"
env:
- name: POD_NAME
valueFrom:
fieldRef:
fieldPath: metadata.name
- name: USER
value: "testuser"
- name: UID
value: "1001"
- name: HOME
value: "/home/testuser"
- name: GROUP
value: "testgroup"
- name: GID
value: "1002"
- name: PATH
value: "/usr/bin:/usr/local/bin"
command:
- "/bin/ls"
- "-lrt"
Expand Down
54 changes: 18 additions & 36 deletions spec/job/adapters/kubernetes/batch_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
Batch = OodCore::Job::Adapters::Kubernetes::Batch
Helper = OodCore::Job::Adapters::Kubernetes::Helper
K8sJobInfo = OodCore::Job::Adapters::Kubernetes::K8sJobInfo
User = Struct.new(:dir, :uid, :gid, keyword_init: true)

let(:helper) {
helper = Helper.new
Expand Down Expand Up @@ -275,16 +276,10 @@ def build_script(opts = {})
image: 'ruby:2.5',
command: 'rake spec',
port: 8080,
env: [
{
name: 'HOME',
value: '/my/home'
},
{
name: 'PATH',
value: '/usr/bin:/usr/local/bin'
}
],
env: {
HOME: '/my/home',
PATH: '/usr/bin:/usr/local/bin'
},
memory: '6Gi',
cpu: '4',
working_dir: '/my/home',
Expand All @@ -311,8 +306,8 @@ def build_script(opts = {})

allow(configured_batch).to receive(:generate_id).with('rspec-test').and_return('rspec-test-123')
allow(configured_batch).to receive(:username).and_return('testuser')
allow(configured_batch).to receive(:run_as_user).and_return(1001)
allow(configured_batch).to receive(:run_as_group).and_return(1002)
allow(configured_batch).to receive(:user).and_return(User.new(dir: '/home/testuser', uid: 1001, gid: 1002))
allow(configured_batch).to receive(:group).and_return('testgroup')

# make sure it get's templated right, also helpful in debugging bc
# it'll show a better diff than the test below.
Expand All @@ -339,16 +334,10 @@ def build_script(opts = {})
image: 'ruby:2.5',
command: 'rake spec',
port: 8080,
env: [
{
name: 'HOME',
value: '/my/home'
},
{
name: 'PATH',
value: '/usr/bin:/usr/local/bin'
}
],
env: {
'HOME' => '/my/home',
'PATH' => '/usr/bin:/usr/local/bin'
},
memory: '6Gi',
cpu: '4',
working_dir: '/my/home',
Expand All @@ -375,8 +364,8 @@ def build_script(opts = {})

allow(@basic_batch).to receive(:generate_id).with('rspec-test').and_return('rspec-test-123')
allow(@basic_batch).to receive(:username).and_return('testuser')
allow(@basic_batch).to receive(:run_as_user).and_return(1001)
allow(@basic_batch).to receive(:run_as_group).and_return(1002)
allow(@basic_batch).to receive(:user).and_return(User.new(dir: '/home/testuser', uid: 1001, gid: 1002))
allow(@basic_batch).to receive(:group).and_return('testgroup')

# make sure it get's templated right, also helpful in debugging bc
# it'll show a better diff than the test below.
Expand All @@ -403,16 +392,9 @@ def build_script(opts = {})
image: 'ruby:2.5',
command: 'rake spec',
port: 8080,
env: [
{
name: 'HOME',
value: '/my/home'
},
{
name: 'PATH',
value: '/usr/bin:/usr/local/bin'
}
],
env: {
PATH: '/usr/bin:/usr/local/bin'
},
memory: '6Gi',
cpu: '4',
working_dir: '/my/home',
Expand All @@ -432,8 +414,8 @@ def build_script(opts = {})

allow(@basic_batch).to receive(:generate_id).with('rspec-test').and_return('rspec-test-123')
allow(@basic_batch).to receive(:username).and_return('testuser')
allow(@basic_batch).to receive(:run_as_user).and_return(1001)
allow(@basic_batch).to receive(:run_as_group).and_return(1002)
allow(@basic_batch).to receive(:user).and_return(User.new(dir: '/home/testuser', uid: 1001, gid: 1002))
allow(@basic_batch).to receive(:group).and_return('testgroup')

# make sure it get's templated right, also helpful in debugging bc
# it'll show a better diff than the test below.
Expand Down
Loading