Support the containers.conf container_name_as_hostname option #5943
Conversation
openshift-ci
bot
added
the
kind/feature
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: nalind 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 |
nalind
force-pushed
the
container_name_as_hostname
branch
3 times, most recently
from
a433090 to
1991aae
Compare
run_common.go
Outdated
| // mapContainerNameToHostname returns the passed-in string with characters that | ||
| // don't match the pattern "[A-Za-z0-9][A-Za-z0-9._-]+" stripped out. | ||
| func mapContainerNameToHostname(containerName string) string { | ||
| const initialAllowedChars = "abcdefghijklmnopqrstuvwxyz" + "ABCDEFGHIJKLMNOPQRSTUVWXYZ" + "0123456789" |
There was a problem hiding this comment.
Would regex such as ^[A-Za-z0-9] and [A-Za-z0-9._-] be easier to maintain ? or could it be slow here ? I mean directly using regexp instead of string matching.
There was a problem hiding this comment.
That's potentially a lot of calls to FindStringIndex(), but it's worth a shot if the result is more readable.
run_common.go
Outdated
| // don't match the pattern "[A-Za-z0-9][A-Za-z0-9._-]+" stripped out. | ||
| func mapContainerNameToHostname(containerName string) string { | ||
| const initialAllowedChars = "abcdefghijklmnopqrstuvwxyz" + "ABCDEFGHIJKLMNOPQRSTUVWXYZ" + "0123456789" | ||
| const allowedChars = initialAllowedChars + "._-" |
There was a problem hiding this comment.
_ is not a valid hostname char and must be stripped, the same is done in podman containers/podman@6fa234a
nalind
force-pushed
the
container_name_as_hostname
branch
from
1991aae to
455f9d0
Compare
Nothing between setting one in the spec and handing it off to the runtime. Added one when we base it on the container name. |
When containers.conf has the "container_name_as_hostname" option set, use that value, with values that don't fit `[A-Za-z0-9][A-Za-z0-9.-]+` stripped out. Signed-off-by: Nalin Dahyabhai <[email protected]>
nalind
force-pushed
the
container_name_as_hostname
branch
from
455f9d0 to
72e2bf4
Compare
What type of PR is this?
/kind feature
What this PR does / why we need it:
When containers.conf has the "container_name_as_hostname" option set, use that value, with values that don't fit
[A-Za-z0-9][A-Za-z0-9._-]+stripped out.How to verify it
New integration test!
Which issue(s) this PR fixes:
None
Special notes for your reviewer:
Does this PR introduce a user-facing change?