allow namespace specification via parameter in templates #12918
Conversation
bparees
force-pushed
the
template_namespace
branch
from
39c9e59 to
684d3aa
Compare
bparees
changed the title
bparees
force-pushed
the
template_namespace
branch
2 times, most recently
from
d259a09 to
2ecca27
Compare
|
[testextended][extended:core(image_ecosystem)] @smarterclayton ptal + sign off on api change |
|
@smarterclayton also weigh in on whether you want to take the risk of putting this in 3.5 because you have plans for it, or hold it until 3.6. |
|
[test] |
pkg/template/template.go
Outdated
| // If an object definition's metadata includes a namespace field, the field will be stripped out of | ||
| // the definition during template instantiation. This is necessary because all objects created during | ||
| // instantiation are placed into the target namespace, so it would be invalid for the object to declare | ||
| //a different namespace. | ||
| stripNamespace(newItem) | ||
| stripNamespace(item) |
There was a problem hiding this comment.
Shouldn't either the comment above be tweaked, or a new comment added after this line, to equally reflect the new caveat of a parameter being able to override, even though the user still cannot explicitly set the namespace.
| "apiVersion": "v1", | ||
| "metadata": { | ||
| "name": "route-edge4", | ||
| "namespace": "prefix-${{SUBSTITUTED}}" |
There was a problem hiding this comment.
Would an additional Route object with a namespace of the form "${SUBSTITUED}-suffix" provide additional test coverage?
There was a problem hiding this comment.
not really, no. i mean "yes it would provide additional coverage" but not of a scenario i'm particularly worried about us breaking. (even doing the prefix testing at all is probably overkill)
test/cmd/newapp.sh
Outdated
| @@ -50,6 +52,23 @@ os::cmd::expect_failure 'oc get dc/mysql' | |||
| os::cmd::expect_failure 'oc get dc/php' | |||
| os::cmd::expect_success_and_text 'oc new-app -f test/testdata/template-without-app-label.json -o yaml' 'app: ruby-helloworld-sample' | |||
|
|
|||
| # check object namespace handling | |||
| # hardcoded values should be stripped | |||
| os::cmd::expect_success_and_not_text 'oc new-app -f test/testdata/template-with-namespaces.json -o yaml' 'namespace: STRIPPED' | |||
There was a problem hiding this comment.
Use of -o jsonpath would be a more precise way to verify the namespace field for all of these new additions.
|
Risk seems low. Add to the Godoc for templates a description of this behavior (how namespace is handled). Also make a comment on the "objects" fied |
bparees
force-pushed
the
template_namespace
branch
from
2ecca27 to
b20ae0f
Compare
I doc'd the objects field to fully explain the namespace handling behavior. Putting it in the template godoc feels redundant. @smarterclayton @gabemontero updates made. |
|
igtm |
pkg/template/api/types.go
Outdated
| @@ -26,6 +26,11 @@ type Template struct { | |||
| Parameters []Parameter | |||
|
|
|||
| // objects is an array of resources to include in this template. | |||
| // If a namespace value is hardcoded in the object, it will be removed | |||
| // during template instantiation, however if the namespace value | |||
| // is, or container, a ${PARAMETER_REFERENCE}, the resolved | |||
|
Lgtm, swagger needs to be regen'd after your typo, let's put this in now since it's low risk and opens the door for templates for infra tools. Add a release note |
|
@smarterclayton typo fixed, will push in a second. Where are we tracking release notes these days? |
bparees
force-pushed
the
template_namespace
branch
from
b20ae0f to
2dfefb2
Compare
|
OpenShift 1.5/3.5 Release Notes Tracker
<openshift/openshift-docs#3404>
…On Wed, Feb 15, 2017 at 3:05 PM, OpenShift Bot ***@***.***> wrote:
continuous-integration/openshift-jenkins/test Running (
https://ci.openshift.redhat.com/jenkins/job/test_pull_
requests_origin_future/238/) (Base Commit: c4879fc
<c4879fc>
)
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#12918 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABG_p7aduMwEyakWiSzXS0f5dPQle4L1ks5rc1qfgaJpZM4L9woa>
.
|
|
Evaluated for origin test up to 2dfefb2 |
|
[merge] |
|
Evaluated for origin testextended up to 2dfefb2 |
|
continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin_future/247/) (Base Commit: a0ea9b5) |
|
continuous-integration/openshift-jenkins/testextended SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin_extended/1110/) (Base Commit: efab088) (Extended Tests: core(image_ecosystem)) |
|
[merge] |
|
flake #12993 |
|
pretty sure this is not a flake, but: [merge] |
|
[merge] |
|
[merge] |
|
(flake #10773) |
|
Flake in router integration @knobuc
On Feb 17, 2017, at 2:29 PM, OpenShift Bot <[email protected]> wrote:
continuous-integration/openshift-jenkins/merge FAILURE (
https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin_future/304/)
(Base Commit: 15e6cf4
<15e6cf4>
)
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#12918 (comment)>,
or mute
the thread
<https://github.com/notifications/unsubscribe-auth/ABG_p5Mn9KjrLa80B3P-Q0g9vkyx0QLMks5rdfUVgaJpZM4L9woa>
.
|
|
flake #9845 |
|
Evaluated for origin merge up to 2dfefb2 |
|
continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin_future/317/) (Base Commit: e4300b3) (Image: devenv-rhel7_5931) |
with this change it's now possible to set a namespace on objects in a template and have them created in that namespace. (prior to this change, any namespace value in the template object was cleared and the objects were created in the target namespace for the template instantiation).
notice that we're intentionally still clearing hardcoded namespace values to provide some level of backwards compatibility, but if the namespace value comes from parameter substitution, we respect the parameter value for the object creation.