Skip to content

helm: use unique names for container ports #5413

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jul 7, 2025
Merged

helm: use unique names for container ports #5413

merged 1 commit into from
Jul 7, 2025

Conversation

@sebhoss
Copy link
Contributor

@sebhoss sebhoss commented Jul 4, 2025

Describe what this PR does

fixes #5399 by changing all container port names to something unique. I've prepended the name of the container in all cases to establish a consistent naming scheme.

Is there anything that requires special attention

No

Related issues

Fixes: #5399

Future concerns

None

Checklist:

  • Commit Message Formatting: Commit titles and messages follow
    guidelines in the developer
    guide    .
  • Reviewed the developer guide on Submitting a Pull
    Request
  • Pending release
    notes
    updated with breaking and/or notable changes for the next major release.
  • Documentation has been updated, if necessary.
  • Unit tests have been added, if necessary.
  • Integration tests have been added, if necessary.
Show available bot commands

These commands are normally not required, but in case of issues, leave any of
the following bot commands in an otherwise empty comment in this PR:

  • /retest ci/centos/<job-name>: retest the <job-name> after unrelated
    failure (please report the failure too!)

@sebhoss sebhoss mentioned this pull request Jul 4, 2025
Closed
@mergify mergify bot added the component/deployment Helm chart, kubernetes templates and configuration Issues/PRs label Jul 4, 2025
Madhu-1
Madhu-1 previously approved these changes Jul 7, 2025
View reviewed changes
Copy link
Collaborator

@Madhu-1 Madhu-1 left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks @sebhoss for the fix.

@Madhu-1 Madhu-1 requested a review from a team July 7, 2025 08:06
nixpanic
nixpanic previously approved these changes Jul 7, 2025
View reviewed changes
@nixpanic
Copy link
Member

nixpanic commented Jul 7, 2025

@Mergifyio rebase

@mergify
Copy link
Contributor

mergify bot commented Jul 7, 2025

rebase

☑️ Nothing to do, the required conditions are not met

  • any of:
    • #commits > 1 [📌 rebase requirement]
    • #commits-behind > 0 [📌 rebase requirement]
    • -linear-history [📌 rebase requirement]
  • -closed [📌 rebase requirement]
  • -conflict [📌 rebase requirement]
  • queue-position = -1 [📌 rebase requirement]

@nixpanic
Copy link
Member

nixpanic commented Jul 7, 2025

@Mergifyio queue

@mergify
Copy link
Contributor

mergify bot commented Jul 7, 2025 •
edited
Loading

queue

🛑 The pull request has been removed from the queue default

The merge conditions cannot be satisfied due to failing checks.

You can take a look at Queue: Embarked in merge queue check runs for more details about the failure.

@mergify mergify bot added the ok-to-test Label to trigger E2E tests label Jul 7, 2025
@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/k8s-e2e-external-storage/1.32

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/k8s-e2e-external-storage/1.33

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/upgrade-tests-cephfs

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e-helm/k8s-1.32

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e-helm/k8s-1.33

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/k8s-e2e-external-storage/1.31

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/upgrade-tests-rbd

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e/k8s-1.32

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e/k8s-1.33

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e-helm/k8s-1.31

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e/k8s-1.31

@ceph-csi-bot ceph-csi-bot removed the ok-to-test Label to trigger E2E tests label Jul 7, 2025
@mergify
Copy link
Contributor

mergify bot commented Jul 7, 2025

This pull request has been removed from the queue for the following reason: checks failed.

The merge conditions cannot be satisfied due to failing checks:

You may have to fix your CI before adding the pull request to the queue again.
If you update this pull request, to fix the CI, it will automatically be requeued once the queue conditions match again.
If you think this was a flaky issue instead, you can requeue the pull request, without updating it, by posting a @mergifyio requeue comment.

@Madhu-1
Copy link
Collaborator

Madhu-1 commented Jul 7, 2025

The Deployment "csi-rbdplugin-provisioner" is invalid:

  • spec.template.spec.containers[1].ports[0].name: Invalid value: "provisioner-http-endpoint": must be no more than 15 characters
  • spec.template.spec.containers[2].ports[0].name: Invalid value: "snapshotter-http-endpoint": must be no more than 15 characters
  • spec.template.spec.containers[3].ports[0].name: Invalid value: "attacher-http-endpoint": must be no more than 15 characters
  • spec.template.spec.containers[4].ports[0].name: Invalid value: "resizer-http-endpoint": must be no more than 15 characters

@sebhoss can you please fix the issue

@mergify mergify bot dismissed stale reviews from Madhu-1 and nixpanic July 7, 2025 13:52

Pull request has been modified.

@sebhoss
Copy link
Contributor Author

sebhoss commented Jul 7, 2025

@Madhu-1 damn sorry - fixed this in the latest commit. I ended up shortening the names so that they mirror the name of the component, e.g. snapshotter-http-endpoint (25 chars) becomes just snapshotter because even snapshotter-http is too long (16 chars).

@Madhu-1 Madhu-1 requested a review from nixpanic July 7, 2025 13:53
@sebhoss
Copy link
Contributor Author

sebhoss commented Jul 7, 2025

I think the test error in go-test / go-test-api and golangci-lint / golangci-lint are network problems?

@Madhu-1
Copy link
Collaborator

Madhu-1 commented Jul 7, 2025

I think the test error in go-test / go-test-api and golangci-lint / golangci-lint are network problems?

Yes correct, i have restart the jobs

@nixpanic
Copy link
Member

nixpanic commented Jul 7, 2025

@Mergifyio queue

@mergify
Copy link
Contributor

mergify bot commented Jul 7, 2025 •
edited
Loading

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at f3dc19d

@mergify mergify bot added the ok-to-test Label to trigger E2E tests label Jul 7, 2025
@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/k8s-e2e-external-storage/1.33

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/k8s-e2e-external-storage/1.31

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/k8s-e2e-external-storage/1.32

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e-helm/k8s-1.33

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e-helm/k8s-1.32

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e-helm/k8s-1.31

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e/k8s-1.33

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e/k8s-1.32

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e/k8s-1.31

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/upgrade-tests-cephfs

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/upgrade-tests-rbd

@ceph-csi-bot ceph-csi-bot removed the ok-to-test Label to trigger E2E tests label Jul 7, 2025
@mergify mergify bot merged commit f3dc19d into ceph:devel Jul 7, 2025
39 of 42 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nixpanic nixpanic nixpanic approved these changes

@Madhu-1 Madhu-1 Madhu-1 approved these changes

Assignees

No one assigned

Labels

component/deployment Helm chart, kubernetes templates and configuration Issues/PRs

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

kubectl diff warns about duplicate port names

4 participants

@sebhoss @nixpanic @ceph-csi-bot @Madhu-1