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

iscsi: set node.startup to manual #57475

Merged
merged 2 commits into from
Jan 2, 2018
Merged

iscsi: set node.startup to manual #57475

merged 2 commits into from
Jan 2, 2018

Conversation

stmcginnis
Copy link
Contributor

If the default iSCSI node.startup is set to automatic, if there is a node failure,
any pods on that node will get rescheduled to another node. If the failed node is
later brought back up it will then try to log back in to any iSCSI sessions it had
prior to the failure, which may no longer exist or may be now in-use by the other
nodes.

It appears most platforms keep the open-iscsi default of node.startup-automatic.
But in case this system-wide setting has been changed, and just to be explicit, this
sets node.startup values for kubernetes controlled volumes to manual.

Fixes #21305

iSCSI sessions managed by kubernetes will now explicitly set startup.mode to 'manual' to
prevent automatic login after node failure recovery. This is the default open-iscsi mode, so
this change will only impact users who have changed their startup.mode to be 'automatic'
in /etc/iscsi/iscsid.conf.

@stmcginnis
iscsi: set node.startup to manual
69a2dee 
If the default iSCSI node.startup is set to automatic, if there is a node failure,
any pods on that node will get rescheduled to another node. If the failed node is
later brought back up it will then try to log back in to any iSCSI sessions it had
prior to the failure, which may no longer exist or may be now in-use by the other
nodes.

It appears most platforms keep the open-iscsi default of node.startup-automatic.
But in case this system-wide setting has been changed, and just to be explicit, this
sets node.startup values for kubernetes controlled volumes to manual.

Closes issue #21305
@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Dec 20, 2017
@dims
Copy link
Member

dims commented Dec 20, 2017

/ok-to-test

@k8s-ci-robot k8s-ci-robot removed the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Dec 20, 2017
@stmcginnis
Copy link
Contributor Author

I realize I maybe should have put in more comments about why I put it per session.

It could do the whole target portal, but I went this route in case they have a mix of manually exposed storage and k8s managed storage. I don't know why they would do that, but the assumption is there was some reason they set the system-wide setting to automatic. This makes it explicitly per session managed by kubernetes.

@stmcginnis
Fixed space/tab indentation
9f97d61 
Some lines had spaces for indentation instead of tabs.
@msau42
Copy link
Member

msau42 commented Dec 23, 2017

/assign @mtanino @rootfs

@rootfs
Copy link
Contributor

rootfs commented Jan 2, 2018

/approve

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 2, 2018
@rootfs
Copy link
Contributor

rootfs commented Jan 2, 2018

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 2, 2018
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rootfs, stmcginnis

Associated issue: #21305

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these OWNERS Files:

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

@stmcginnis
Copy link
Contributor Author

/test pull-kubernetes-unit

@k8s-github-robot
Copy link

Automatic merge from submit-queue (batch tested with PRs 57399, 57751, 57475). If you want to cherry-pick this change to another branch, please follow the instructions here.

@k8s-github-robot k8s-github-robot merged commit 462db17 into kubernetes:master Jan 2, 2018
@stmcginnis stmcginnis deleted the iscsi_node_startup branch January 3, 2018 06:56
@kincl kincl mentioned this pull request Jun 26, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@humblec humblec

@rootfs rootfs

@mtanino mtanino

@msau42 msau42

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", 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/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@stmcginnis @dims @msau42 @rootfs @k8s-ci-robot @k8s-github-robot @humblec @mtanino