OPECO-2735: initial update to avoid nested docker execution#1075
Conversation
Signed-off-by: Jordan Keister <jordan@nimblewidget.com>
openshift-ci-robot
added
the
jira/valid-reference
|
@grokspawn: This pull request references OPECO-2735 which is a valid jira issue. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
openshift-ci
Bot
added
the
approved
|
/hold while sorting out utest issues. |
openshift-ci
Bot
added
the
do-not-merge/hold
Codecov Report
@@ Coverage Diff @@
## master #1075 +/- ##
==========================================
- Coverage 52.75% 52.58% -0.17%
==========================================
Files 106 107 +1
Lines 9378 9384 +6
==========================================
- Hits 4947 4935 -12
- Misses 3517 3522 +5
- Partials 914 927 +13
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
grokspawn
force-pushed
the
composite-private-registries
branch
from
5ef6c33 to
9a636a8
Compare
|
|
||
| func (rb *RawBuilder) Validate(dir string) error { | ||
| return validate(rb.builderCfg.ContainerCfg, path.Join(rb.builderCfg.CurrentDirectory, dir)) | ||
| return validate(rb.builderCfg.ContainerCfg, path.Join(rb.builderCfg.InputDirectory, dir)) |
There was a problem hiding this comment.
This namechange really illustrates how moving from container-in-container to container-less really moves us in the direction of re-examining the entire schema.
Logically, this should be builderCfg.ContainerCfg.WorkingDir unless CompositeConfig.Components[].Destination.Path (dir here) is canonical, in which case builderCfg.InputDirectory is superfluous.
I think we need to eliminate the WorkingDir element from the schema since it only really makes sense w/in a container.
Similar for InputDirectory where we really need to be able to rely on the input path from the author.
Still nibbling on this, but might just choose to leave as-is for this PR.
There was a problem hiding this comment.
On later thought, WorkingDir makes a lot of sense from the use-case from the maintainer of "I will look for contributions generated here".
I think InputDirectory is still obsolete, though. We'll remove and adjust the schema for fit in a later PR.
Signed-off-by: Jordan Keister <jordan@nimblewidget.com>
grokspawn
force-pushed
the
composite-private-registries
branch
from
9a636a8 to
37817e3
Compare
|
/hold cancel |
openshift-ci
Bot
removed
the
do-not-merge/hold
everettraven
left a comment
everettraven
left a comment
There was a problem hiding this comment.
Looks good to me overall. Only nits I have is leaving in the BuilderConfig.InputDirectory field if it isn't going to be used and maybe simplifying the BuilderConfig struct by removing the ContainerConfig struct since we only use a string field from it. I'm OK with it going in like this as long as the plan is to clean it up in follow up PRs.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: everettraven, grokspawn The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/lgtm |
5 participants
Description of the change:
Motivation for the change:
Reviewer Checklist
/docs