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

Enable host networking for provisioner #3177

Merged
merged 1 commit into from
Jul 4, 2022

Conversation

iceman91176
Copy link
Contributor

Describe what this PR does

Adds the possibility in the helm-chart to enable hostNetworking. Useful for cases where the podNetwork has no access to ceph, but the host has.

Is there anything that requires special attention

No ;-)

Related issues

Fixes: #3167


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!)
  • /retest all: run this in case the CentOS CI failed to start/report any test
    progress or results

@yati1998
Copy link
Contributor

@iceman91176 please check the Commit guidelines to pass the CI.

@iceman91176 iceman91176 force-pushed the feat_helm_hostnetworking branch 2 times, most recently from c8f22b5 to aa49421 Compare June 13, 2022 14:24
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.

@iceman91176 Thanks for the contribution, can you please remove extra commits which is not related to this one.

@@ -164,6 +164,10 @@ provisioner:
# system-cluster-critical which is less priority than system-node-critical
priorityClassName: system-cluster-critical

# enable hostnetwork for provisioner pod. default is false
# useful for deployments where the podNetwork has no access to ceph
enableHostNetwork: false
Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hm, i don't know where that additional commit came from. I'll take a look at it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Did as suggested. Way simpler..

@Madhu-1 Madhu-1 added the component/deployment Helm chart, kubernetes templates and configuration Issues/PRs label Jun 14, 2022
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.

changes LGTM. can you please squash the commits into 1? Sadly we don't have CI to test helm upgrades, did you get a chance to verify the helm upgrade (cephcsi) from the older version to a version for this change?

@iceman91176
Copy link
Contributor Author

Well, i performed a dry-run for install and upgrade . That looked good to me.
Commits have been squashed

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.

You might need to update commit message as below

helm: enable host networking for provisioner

Adds the possibility in the helm-chart to enable hostNetworking
for provider pods.

Signed-off-by: Carsten Buchberger <c.buchberger@witcom.de>

@nixpanic nixpanic requested review from Madhu-1 and a team June 17, 2022 11:37
@Rakshith-R
Copy link
Contributor

@Mergifyio rebase

@Rakshith-R Rakshith-R added the ci/retry/e2e Label to retry e2e retesting on approved PR's label Jul 4, 2022
Adds the possibility in the helm-chart to enable hostNetworking
for provider pods.

Signed-off-by: Carsten Buchberger <c.buchberger@witcom.de>
@mergify
Copy link
Contributor

mergify bot commented Jul 4, 2022

rebase

✅ Branch has been successfully rebased

@ceph-csi-bot
Copy link
Collaborator

/retest ci/centos/mini-e2e/k8s-1.21

@ceph-csi-bot
Copy link
Collaborator

@iceman91176 "ci/centos/mini-e2e/k8s-1.21" test failed. Logs are available at location for debugging

@ceph-csi-bot
Copy link
Collaborator

@Mergifyio requeue

@mergify
Copy link
Contributor

mergify bot commented Jul 4, 2022

requeue

❌ This pull request head commit has not been previously disembarked from queue.

@mergify mergify bot merged commit b262f06 into ceph:devel Jul 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci/retry/e2e Label to retry e2e retesting on approved PR's component/deployment Helm chart, kubernetes templates and configuration Issues/PRs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Helm option to to enable host networking for provisioner (hostNetwork: true)
6 participants