Update test to run on OpenShift #350
Conversation
Signed-off-by: Kevin Earls <[email protected]>
|
This change is |
Codecov Report
@@ Coverage Diff @@
## master #350 +/- ##
=======================================
Coverage 89.88% 89.88%
=======================================
Files 64 64
Lines 3035 3035
=======================================
Hits 2728 2728
Misses 207 207
Partials 100 100Continue to review full report at Codecov.
|
|
@jpkrohling @objectiser Please review. |
There was a problem hiding this comment.
Reviewed 2 of 2 files at r1.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @kevinearls)
test/e2e/daemonset.go, line 50 at r1 (raw file):
} cmd := exec.Command("oc", "adm", "--namespace", namespace, "policy", "add-scc-to-user", "daemonset-with-hostport", "-z", "default")
Could you add a code comment, explaining your choice? Like, "ideally, we would use the REST API, but for a single-usage within the project, this is the simplest solution that works".
test/e2e/daemonset.go, line 214 at r1 (raw file):
} func hostportSccDaemonset() (*osv1sec.SecurityContextConstraints) {
s/hostport/hostPort/?
…et method Signed-off-by: Kevin Earls <[email protected]>
There was a problem hiding this comment.
Reviewed 1 of 1 files at r2.
Reviewable status:complete! all files reviewed, all discussions resolved
|
@objectiser @jpkrohling Can one of you guys merge this? Juca I assume the code-review/reviewable failure is because you turned that off? |
|
That's correct. Sorry I didn't merge it earlier, I was holding it off until I executed the tests locally and ran into #355, but I assume you tested it as well. |
Signed-off-by: Kevin Earls [email protected]
This resolves #327 by taking actions that are needed to permit this test to run on OpenShift. Part of the solution is a bit hacky as it uses os.Exec to call the "oc adm" command, which means the oc client needs to be on the path wherever this test is being run.
It would be preferable to use either the OpenShift Go client of the Rest API to do this, but we don't use those anywhere else in the tests and adding them would be non-trivial. I will open an issue to do this in the future.