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.

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

Add default containerPort for cdi-apiserver deployment #3465

Merged
merged 1 commit into from
Oct 24, 2024
Merged

Add default containerPort for cdi-apiserver deployment #3465

merged 1 commit into from
Oct 24, 2024

Conversation

scoquelin
Copy link
Contributor

@scoquelin scoquelin commented Oct 17, 2024 •
edited
Loading

What this PR does / why we need it:

We have a use case where we run kubevirt/cdi with HostNetwork=true

This PR is just adding containerPort explicitly when creating the deployment for cdi-apiserver so that Kubernetes can pro-actively detect if a pod needs to be assigned to other nodes in case the port is already taken.

Special notes for your reviewer:

Release note:

Explicitly declaring containerPort in cdi-apiserver deployment

@kubevirt-bot kubevirt-bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. dco-signoff: no Indicates the PR's author has not DCO signed all their commits. labels Oct 17, 2024
@kubevirt-bot
Copy link
Contributor

Hi @scoquelin. Thanks for your PR.

PRs from untrusted users cannot be marked as trusted with /ok-to-test in this repo meaning untrusted PR authors can never trigger tests themselves. Collaborators can still trigger tests on the PR using /test all.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@scoquelin scoquelin force-pushed the api-server-container-port branch from f6de4cc to cb34b43 Compare October 18, 2024 03:07
@kubevirt-bot kubevirt-bot added dco-signoff: yes Indicates the PR's author has DCO signed all their commits. and removed dco-signoff: no Indicates the PR's author has not DCO signed all their commits. labels Oct 18, 2024
@awels
Copy link
Member

awels commented Oct 24, 2024

/test all

@awels
Copy link
Member

awels commented Oct 24, 2024

Sorry I haven't gotten around to taking a look at this sooner. I don't think we always want to add the node port to the container, only in certain cases. Maybe create a feature gate to do it. Or a configuration field in CDIConfig. Have to ponder that a little bit. @mhenriks what do you think?

@mhenriks
Copy link
Member

I'm fine with this

/lgtm

@kubevirt-bot kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Oct 24, 2024
@coveralls
Copy link

Coverage Status

coverage: 59.299% (+0.1%) from 59.196%
when pulling cb34b43 on scoquelin:api-server-container-port
into 5f66979 on kubevirt:main.

@awels
Copy link
Member

awels commented Oct 24, 2024

/approve

@kubevirt-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: awels

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

@kubevirt-bot kubevirt-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 24, 2024
@awels
Copy link
Member

awels commented Oct 24, 2024

/test pull-containerized-data-importer-e2e-upg

@kubevirt-bot kubevirt-bot merged commit cc27c84 into kubevirt:main Oct 24, 2024
20 checks passed
@scoquelin
Copy link
Contributor Author

Thank you all! 🥳

@scoquelin scoquelin deleted the api-server-container-port branch October 24, 2024 22:25
universal-itengineer pushed a commit to deckhouse/3p-containerized-data-importer that referenced this pull request Oct 29, 2024
@scoquelin @universal-itengineer
Add default containerPort for cdi-apiserver deployment (kubevirt#3465)
369e988 
Signed-off-by: Sebastien Coquelin <[email protected]>
universal-itengineer pushed a commit to deckhouse/3p-containerized-data-importer that referenced this pull request Oct 29, 2024
@scoquelin @universal-itengineer
Add default containerPort for cdi-apiserver deployment (kubevirt#3465)
24cfaf5 
Signed-off-by: Sebastien Coquelin <[email protected]>
mhenriks added a commit to mhenriks/containerized-data-importer that referenced this pull request Oct 31, 2024
@mhenriks
Make sure TerminationMessagePolicy/Path are set when creating cdi-api…
ff403fd 
…server

Fixing regression introduced in kubevirt#3465

Signed-off-by: Michael Henriksen <[email protected]>
@mhenriks mhenriks mentioned this pull request Oct 31, 2024
Merged
kubevirt-bot pushed a commit that referenced this pull request Oct 31, 2024
@mhenriks
Make sure TerminationMessagePolicy/Path are set when creating cdi-api…
d9b91dc 
…server (#3489)

Fixing regression introduced in #3465

Signed-off-by: Michael Henriksen <[email protected]>
kubevirt-bot pushed a commit to kubevirt-bot/containerized-data-importer that referenced this pull request Oct 31, 2024
@mhenriks
Make sure TerminationMessagePolicy/Path are set when creating cdi-api…
e040c25 
…server

Fixing regression introduced in kubevirt#3465

Signed-off-by: Michael Henriksen <[email protected]>
kubevirt-bot added a commit that referenced this pull request Oct 31, 2024
@kubevirt-bot @mhenriks
Make sure TerminationMessagePolicy/Path are set when creating cdi-api…
bf5dd2f 
…server (#3498)

Fixing regression introduced in #3465

Signed-off-by: Michael Henriksen <[email protected]>
Co-authored-by: Michael Henriksen <[email protected]>
@scoquelin scoquelin mentioned this pull request Nov 29, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@awels awels Awaiting requested review from awels

@mhenriks mhenriks Awaiting requested review from mhenriks

Assignees

@mhenriks mhenriks

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. lgtm Indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/S
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@scoquelin @kubevirt-bot @awels @mhenriks @coveralls