Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: use endpoint names as container port names if they shorter than 16 characters #736

Merged
merged 2 commits into from
Jan 17, 2022

Conversation

amisevsk
Copy link
Collaborator

What does this PR do?

Reuse endpoint name as container port name if it is short enough (<=15 chars). Otherwise, use current format (<portnum>-<protocol>).

What issues does this PR fix or reference?

Container port names can be confusing

Is it tested? How?

Test cases are added. Functionality should not change as workspace services refer to ports by number and not name.

PR Checklist

  • E2E tests pass (when PR is ready, comment /test v8-devworkspace-operator-e2e, v8-che-happy-path to trigger)
    • v8-devworkspace-operator-e2e: DevWorkspace e2e test
    • v8-che-happy-path: Happy path for verification integration with Che

Additional info

Reuse the endpoint name as the port name in a workspace container if it
fits the pod spec for a port name (max length = 15 chars). If an endpoint
name is longer than 15 chars, fallback to previous functionality of
formatting <portNum>-<protocol>. In the case of duplicated ports in one
container with short names, use the name from the first port.

Signed-off-by: Angel Misevski <[email protected]>
Copy link
Contributor

@ibuziuk ibuziuk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍
just need to fix the semantic pull request check

@openshift-ci
Copy link

openshift-ci bot commented Jan 14, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: amisevsk, ibuziuk

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@amisevsk amisevsk changed the title Pod port names feat: use endpoint names as container port names if they shorter than 16 characters Jan 14, 2022
@amisevsk
Copy link
Collaborator Author

test v8-devworkspace-operator-e2e

@benoitf
Copy link
Collaborator

benoitf commented Jan 17, 2022

/test v8-devworkspace-operator-e2e, v8-che-happy-path

@amisevsk amisevsk merged commit 4b7b81c into devfile:main Jan 17, 2022
@amisevsk amisevsk deleted the pod-port-names branch January 17, 2022 16:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants