-
Notifications
You must be signed in to change notification settings - Fork 72
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
Updates to testsuite according to cnf-testsuite/helm#6 #2218
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Rafal Lal <[email protected]>
Signed-off-by: Rafal Lal <[email protected]>
Signed-off-by: Rafal Lal <[email protected]>
Signed-off-by: Rafal Lal <[email protected]>
e002060
to
0339ddd
Compare
Signed-off-by: Rafal Lal <[email protected]>
0339ddd
to
dbf5630
Compare
Signed-off-by: Rafal Lal <[email protected]>
4f3e897
to
baac78a
Compare
@@ -4,6 +4,6 @@ deployments: | |||
helm_charts: | |||
- name: jenkins | |||
helm_chart_name: jenkins | |||
helm_values: --set controller.sidecars.configAutoReload.enabled=false | |||
helm_values: --set controller.sidecars.configAutoReload.enabled=false --set controller.installPlugins=false |
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.
Jenkins consistently failed to install (error in init container during plugins installation).
@@ -23,7 +23,7 @@ module CNFInstall | |||
begin | |||
CNFManager.ensure_namespace_exists!(helm_namespace) | |||
#TODO (kosstennbl) fix Helm install to add -n to namespace and remove it there | |||
response = Helm.install(release_name: @deployment_name, helm_chart: chart_path, namespace: "-n #{helm_namespace}", values: helm_values) | |||
response = Helm.install(@deployment_name, chart_path, namespace: "#{helm_namespace}", values: helm_values) |
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.
Unnecessary "#{helm_namespace}", just pass the variable
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.
Will fix once approved to go in, dont want to restart long CI.
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.
I don't think that it can be approved before review comments are addressed.
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.
Well, what I meant is when all the involved people will leave theirs comments.
@@ -21,7 +21,7 @@ module FluentManager | |||
Helm.helm_repo_add(flavor_name, repo_url) | |||
File.write(values_file, values_macro) | |||
begin | |||
Helm.install("--values #{values_file} -n #{TESTSUITE_NAMESPACE} #{flavor_name} #{chart}") | |||
Helm.install("#{flavor_name}", "#{chart}", namespace: TESTSUITE_NAMESPACE, values: "--values #{values_file}") |
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.
Unnecessary #{}.
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.
Will fix once approved to go in, dont want to restart long CI.
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.
lgtm
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.
other than:
- incorporating Slavo's comments
- changing dependencies once helm changes are merged and a new helm lib version is relased
-> lgtm.
Description
Updates to testsuite according to helm shard refactor (changes can be found here cnf-testsuite/helm#6). No other changes added besides removing duplicate from
shards.yaml
file.Issues:
Refs: #2199
How has this been tested:
Types of changes:
Checklist:
Documentation
Code Review
Issue