-
Notifications
You must be signed in to change notification settings - Fork 49
Conversation
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: JoshVanL The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
d27ed93
to
ce0bbad
Compare
/unassign @simonswine |
d6800ba
to
e68f762
Compare
Add vendoring Use environment destroy Fixing vendor packages Add e2e test command to makefile Fix environment destroy Add tarmak upgrade test Fix go fmt Fix makefile Add kubernetes upgrade test Refactor code a bit Fix go fmt Fix return statement Refactor duplicate code Get e2e versions programmatically Signed-off-by: Mattias Gees <mattias.gees@gmail.com> Signed-off-by: JoshVanL <vleeuwenjoshua@gmail.com>
e68f762
to
558f674
Compare
/hold |
/hold cance |
/hold cancel |
/unassign |
7853336
to
d3c5f15
Compare
Signed-off-by: JoshVanL <vleeuwenjoshua@gmail.com>
d3c5f15
to
3fc4919
Compare
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.
Makefile
Outdated
@@ -291,3 +291,21 @@ docker_%: | |||
|
|||
# remove container | |||
docker rm $(CONTAINER_ID) | |||
|
|||
e2e-test-all-conformance: build |
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.
Can you add some help text comment so it shows up in make help
?
cmd/tarmak/e2e/sonobuoy_test.go
Outdated
} | ||
|
||
if _, err := os.Stat(si.binPath); os.IsNotExist(err) { | ||
t.Fatal("sonobuoy binary not exissing: ", si.binPath) |
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.
existing
Signed-off-by: JoshVanL <vleeuwenjoshua@gmail.com>
/unassign |
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.
Can you give me more insight why all those changes are necessary?. They seem to be quite high risk that short before release
@@ -26,7 +26,7 @@ | |||
"ssh_username": "centos", | |||
"ssh_pty": "true", | |||
"encrypt_boot": "{{user `ebs_volume_encrypted`}}", | |||
"ami_name": "Tarmak CentOS 7 x86_64 with puppet-agent and latest upstream kernel{{isotime \"2006-01-02_030405\"}}", | |||
"ami_name": "{{user `tarmak_environment`}} - Tarmak CentOS 7 x86_64 with puppet-agent and latest upstream kernel{{isotime \"2006-01-02_030405\"}}", |
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.
Is that change necessary?
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.
No, change came from rebase, will revert
@@ -26,7 +26,7 @@ | |||
"ssh_username": "centos", | |||
"ssh_pty": "true", | |||
"encrypt_boot": "{{user `ebs_volume_encrypted`}}", | |||
"ami_name": "Tarmak CentOS 7 x86_64 with puppet-agent {{isotime \"2006-01-02_030405\"}}", | |||
"ami_name": "{{user `tarmak_environment`}} - Tarmak CentOS 7 x86_64 with puppet-agent {{isotime \"2006-01-02_030405\"}}", |
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.
Is that change necessary?
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.
No, change came from rebase, will revert
@@ -31,18 +31,18 @@ | |||
$post_1_10 = versioncmp($::kubernetes::version, '1.10.0') >= 0 | |||
|
|||
if $post_1_10 { | |||
$app_name = 'core-dns' |
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.
Is that name change down to sonobouy?
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.
Yes, guess we should consider it source of truth
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 am sure kube-dns is the correct name for it though
/assign @JoshVanL |
Signed-off-by: JoshVanL <vleeuwenjoshua@gmail.com>
/unassign |
/lgtm |
/assign @simonswine
fixes #434