-
Notifications
You must be signed in to change notification settings - Fork 4k
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
chore(eks): kubectl 1.16.8-eks-e16311, aws-cli 1.18.86, helm 3.2.4 #8739
Changes from 4 commits
a136fba
d992e95
d70a9e7
fe8ecaa
b92defc
bf31cd1
6bff0c6
0695277
8a81821
dbfeaf5
ff0209f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -53,6 +53,12 @@ export interface HelmChartOptions { | |
* @default Duration.minutes(5) | ||
*/ | ||
readonly timeout?: Duration; | ||
|
||
/** | ||
* create namespace if not exist | ||
* @default False | ||
*/ | ||
readonly createNamespace?: boolean; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Feels like it makes more sense for this to be enabled by default, no? What are the implications? |
||
} | ||
|
||
/** | ||
|
@@ -104,6 +110,7 @@ export class HelmChart extends Construct { | |
Values: (props.values ? stack.toJsonString(props.values) : undefined), | ||
Namespace: props.namespace ?? 'default', | ||
Repository: props.repository, | ||
CreateNamespace: props.createNamespace ?? false, | ||
}, | ||
}); | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,6 +12,7 @@ | |
outdir = os.environ.get('TEST_OUTDIR', '/tmp') | ||
kubeconfig = os.path.join(outdir, 'kubeconfig') | ||
|
||
|
||
def helm_handler(event, context): | ||
logger.info(json.dumps(event)) | ||
|
||
|
@@ -20,22 +21,23 @@ def helm_handler(event, context): | |
|
||
# resource properties | ||
cluster_name = props['ClusterName'] | ||
role_arn = props['RoleArn'] | ||
release = props['Release'] | ||
chart = props['Chart'] | ||
version = props.get('Version', None) | ||
wait = props.get('Wait', False) | ||
timeout = props.get('Timeout', None) | ||
namespace = props.get('Namespace', None) | ||
repository = props.get('Repository', None) | ||
values_text = props.get('Values', None) | ||
role_arn = props['RoleArn'] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please avoid aesthetic changes, those make the code review harder. Any reason spacing was changed? |
||
release = props['Release'] | ||
chart = props['Chart'] | ||
version = props.get('Version', None) | ||
wait = props.get('Wait', False) | ||
timeout = props.get('Timeout', None) | ||
namespace = props.get('Namespace', None) | ||
create_namespace = props.get('CreateNamespace', None) | ||
repository = props.get('Repository', None) | ||
values_text = props.get('Values', None) | ||
|
||
# "log in" to the cluster | ||
subprocess.check_call([ 'aws', 'eks', 'update-kubeconfig', | ||
'--role-arn', role_arn, | ||
'--name', cluster_name, | ||
'--kubeconfig', kubeconfig | ||
]) | ||
subprocess.check_call(['aws', 'eks', 'update-kubeconfig', | ||
'--role-arn', role_arn, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ditto |
||
'--name', cluster_name, | ||
'--kubeconfig', kubeconfig | ||
]) | ||
|
||
# Write out the values to a file and include them with the install and upgrade | ||
values_file = None | ||
|
@@ -46,21 +48,26 @@ def helm_handler(event, context): | |
f.write(json.dumps(values, indent=2)) | ||
|
||
if request_type == 'Create' or request_type == 'Update': | ||
helm('upgrade', release, chart, repository, values_file, namespace, version, wait, timeout) | ||
helm('upgrade', release, chart, repository, values_file, namespace, version, wait, timeout, create_namespace) | ||
elif request_type == "Delete": | ||
try: | ||
helm('uninstall', release, namespace=namespace, timeout=timeout) | ||
except Exception as e: | ||
logger.info("delete error: %s" % e) | ||
|
||
def helm(verb, release, chart = None, repo = None, file = None, namespace = None, version = None, wait = False, timeout = None): | ||
|
||
def helm( | ||
verb, release, chart=None, repo=None, file=None, namespace=None, version=None, wait=False, timeout=None, | ||
create_namespace=None): | ||
import subprocess | ||
|
||
cmnd = ['helm', verb, release] | ||
if not chart is None: | ||
cmnd.append(chart) | ||
if verb == 'upgrade': | ||
cmnd.append('--install') | ||
if not create_namespace is None: | ||
cmnd.append('--create-namespace') | ||
if not repo is None: | ||
cmnd.extend(['--repo', repo]) | ||
if not file is None: | ||
|
@@ -72,9 +79,11 @@ def helm(verb, release, chart = None, repo = None, file = None, namespace = None | |
if wait: | ||
cmnd.append('--wait') | ||
if not timeout is None: | ||
cmnd.extend(['--timeout', timeout]) | ||
cmnd.extend(['--timeout', timeout]) | ||
cmnd.extend(['--kubeconfig', kubeconfig]) | ||
|
||
logger.info(cmnd) | ||
|
||
retry = 3 | ||
while retry > 0: | ||
try: | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry but this should be a separate PR :-)
[think about the changelog entry :-)]
I added comments on the namespace change as well here so it's PR can start from there