container image field values irrelevant with ict's in automatic mode,…

[gabemontero]

… don't have setting that confuses users; add updated quickstarts

@bparees PTAL

[bparees]

Lgtm [merge]

[openshift-bot]

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_requests_origin/5922/) (Image: devenv-rhel7_4212)

[openshift-bot]

Evaluated for origin merge up to f7f5dcf

[openshift-bot]

[Test]ing while waiting on the merge queue

[openshift-bot]

continuous-integration/openshift-jenkins/test Running (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/3856/)

[openshift-bot]

Evaluated for origin test up to f7f5dcf

[rhcarvalho]

I was puzzled when I saw the value set to " ", I'd expect it to be a validation error because a space cannot be a valid image name. Turns out validation doesn't cover that, only requires the string to be non-empty. Isn't it an overlook in the validation code?

[bparees]

1. I thought @gabemontero had made some changes to the deplymentconfig validator to pass a "fake" (valid) value to the k8s validation logic so that DCs would tolerate otherwise invalid image field values in their pod template definition. @gabemontero did that change end up being necessary, or the " " was a workaround to not make that change?
2. should the k8s code validate image field values more strictly? perhaps, that seems like an issue to open/discuss in k8s though. (bearing in mind that if that code does change, and depending on @gabemontero's response to (1), we will have to go update all these templates again).

[gabemontero]

On Fri, May 27, 2016 at 9:11 AM, Ben Parees notifications@github.com
wrote:

1. I thought @gabemontero https://github.com/gabemontero had made some
   changes to the deplymentconfig validator to pass a "fake" (valid) value to
   the k8s validation logic so that DCs would tolerate otherwise invalid image
   field values in their pod template definition. @gabemontero
   https://github.com/gabemontero did that change end up being necessary,
   or the " " was a workaround to not make that change?

At first, the single space was deemed an acceptable placeholder to faking
out the k8s validation logic. But then, as I recall @bparees, you were
concerned about backwards compatibility with the templates with older
versions of V3.x OpenShift, as an empty string or missing images: field
would fail on earlier versions.
In the end, as I understood it, we wanted to still do the fake out of the
validation logic, so customer could take advantage with their templates of
putting empty string for that value when ICTs were present with automatic
true, but for our example/sample templates, they would remain with a single
space for that value. It would satisfy the backward compatibility concern,
but be the "least confusing", as the old dummy values did not properly
convey that their value would ignored, and it was decided that a single
space was better than "ignoreThisValue" or some such.

I do have a prototype of the "fake out" in one of my branches, but it needs
some more polish (dealing with the parameters section technically being
optional for example), and given the above, I've prioritized other work.

2. should the k8s code validate image field values more strictly? perhaps,
   that seems like an issue to open/discuss in k8s though. (bearing in mind
   that if that code does change, and depending on @gabemontero
   https://github.com/gabemontero's response to (1), we will have to go
   update all these templates again).

Yeah that got suggested early on when this came up, but the motivation for
initiating such a discussion appeared low.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#8892 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/ADbadFf8_PixnZYRawuRbUF0fZywdaQ-ks5qFu1ogaJpZM4IfhYS
.

[bparees]

@gabemontero thanks for the refresher, that all sounds right :)

so yeah, again we could bring this up w/ k8s, but we'd be shooting ourselves in the foot since if the k8s logic changes, these templates will be broken (until we either update these templates to some other value, or introduce fakery in the DC validation logic so the DC tolerates a " " value even though the underlying podtemplate validation logic doesn't).