Commit Graph

78 Commits

Author SHA1 Message Date
杨军10092085
361e4ff0fa volume: use contextual logging 2023-03-14 08:37:30 +08:00
Hemant Kumar
a99466ca86 check existing size before querying new size from api-server 2022-03-28 11:32:49 -04:00
Hemant Kumar
ed217f4140 rename SetVolumeSize to InitializeVolumeSize 2022-03-28 11:32:49 -04:00
Hemant Kumar
7a43406138 Do not update PVC if it already has updated size 2022-03-28 11:32:49 -04:00
Hemant Kumar
e4f62d6c41 Modify code to use new interface functions 2022-03-28 11:32:49 -04:00
Jean-Francois Remy
e83184568d Add unit tests
- actual_state_of_world_test.go: test the new method GetVolumesToReportAttachedForNode
  for an existing node and a non-existing node
- node_status_updater_test.go: test UpdateNodeStatuses and UpdateNodeStatuses in nominal
  case with 2 nodes getting one volume each. Test UpdateNodeStatuses with the first call
  to node.patch failing but the following one succeeding
- add comment in node_status_updater.go
- fix log line in reconciler.go
- rename variable in actual_state_of_world.go
2022-02-22 12:21:58 -08:00
Jean-Francois Remy
f1717baaaa Fix nodes volumesAttached status not updated
The UpdateNodeStatuses code stops too early in case there is
an error when calling updateNodeStatus. It will return immediately
which means any remaining node won't have its update status put back
to true.

Looking at the call sites for UpdateNodeStatuses, it appears this is
not the only issue. If the lister call fails with anything but a Not Found
error, it's silently ignored which is wrong in the detach path.
Also the reconciler detach path calls UpdateNodeStatuses but the real intent
is to only update the node currently processed in the loop and not proceed
with the detach call if there is an error updating that specifi node volumesAttached
property. With the current implementation, it will not proceed if there is
an error updating another node (which is not completely bad but not ideal) and
worse it will proceed if there is a lister error on that node which means the
node volumesAttached property won't have been updated.

To fix those issues, introduce the following changes:
- [node_status_updater] introduce UpdateNodeStatusForNode which does what
  UpdateNodeStatuses does but only for the provided node
- [node_status_updater] if the node lister call fails for anything but a Not
  Found error, we will return an error, not ignore it
- [node_status_updater] if the update of a node volumesAttached properties fails
  we continue processing the other nodes
- [actual_state_of_world] introduce GetVolumesToReportAttachedForNode which
  does what GetVolumesToReportAttached but for the node whose name is provided
  it returns a bool which indicates if the node in question needs an update as
  well as the volumesAttached list. It is used by UpdateNodeStatusForNode
- [actual_state_of_world] use write lock in updateNodeStatusUpdateNeeded, we're
  modifying the map content
- [reconciler] use UpdateNodeStatusForNode in the detach loop
2022-02-22 12:20:53 -08:00
Jing Xu
69b9f9b1f0 Fix issue in node status updating VolumeAttached list
During volume detach, the following might happen in reconciler

1. Pod is deleting
2. remove volume from reportedAsAttached, so node status updater will
update volumeAttached list
3. detach failed due to some issue
4. volume is added back in reportedAsAttached
5. reconciler loops again the volume, remove volume from
reportedAsAttached
6. detach will not be trigged because exponential back off, detach call
will fail with exponential backoff error
7. another pod is added which using the same volume on the same node
8. reconciler loops and it will NOT try to tigger detach anymore

At this point, volume is still attached and in actual state, but
volumeAttached list in node status does not has this volume anymore, and
will block volume mount from kubelet.

The fix in first round is to add volume back into the volume list that
need to reported as attached at step 6 when detach call failed with
error (exponentical backoff). However this might has some performance
issue if detach fail for a while. During this time, volume will be keep
removing/adding back to node status which will cause a surge of API
calls.

So we changed to logic to check first whether operation is safe to retry which
means no pending operation or it is not in exponentical backoff time
period before calling detach. This way we can avoid keep removing/adding
volume from node status.

Change-Id: I5d4e760c880d72937d34b9d3e904ecad125f802e
2021-10-05 09:44:35 -07:00
Benjamin Elder
56e092e382 hack/update-bazel.sh 2021-02-28 15:17:29 -08:00
Jayasekhar Konduru
9b2b73600d Recover CSI volumes from dangling attachments
Change-Id: I72105d67d8a4069ab19bfa4638a7ac365cf4194c
2020-12-11 18:31:53 -08:00
Cheng Xing
d9a629fe3a IsVolumeAttachedToNode() renamed to GetAttachState(), and returns 3 states instead of combining "uncertain" and "detached" into "false" 2020-10-29 13:24:51 -07:00
Cheng Xing
a61743b125 Fixes Attach Detach Controller reconciler race reading ActualStateOfWorld and operation pending states; fixes reconciler_test mock detach to account for multiple attaches on a node 2020-10-27 23:51:55 -07:00
Davanum Srinivas
07d88617e5
Run hack/update-vendor.sh
Signed-off-by: Davanum Srinivas <davanum@gmail.com>
2020-05-16 07:54:33 -04:00
Davanum Srinivas
442a69c3bd
switch over k/k to use klog v2
Signed-off-by: Davanum Srinivas <davanum@gmail.com>
2020-05-16 07:54:27 -04:00
taesun_lee
79680b5d9b Fix pkg/controller typos in some error messages, comments etc
- applied review results by LuisSanchez
- Co-Authored-By: Luis Sanchez <sanchezl@redhat.com>

genernal -> general
iniital -> initial
initalObjects -> initialObjects
intentionaly -> intentionally
inforer -> informer
anotother -> another
triger -> trigger
mutli -> multi
Verifyies -> Verifies
valume -> volume
unexpect -> unexpected
unfulfiled -> unfulfilled
implenets -> implements
assignement -> assignment
expectataions -> expectations
nexpected -> unexpected
boundSatsified -> boundSatisfied
externel -> external
calcuates -> calculates
workes -> workers
unitialized -> uninitialized
afater -> after
Espected -> Expected
nodeMontiorGracePeriod -> NodeMonitorGracePeriod
estimateGrracefulTermination -> estimateGracefulTermination
secondrary -> secondary
ShouldRunDaemonPodOnUnscheduableNode -> ShouldRunDaemonPodOnUnschedulableNode
rrror -> error
expectatitons -> expectations
foud -> found
epackage -> package
succesfulJobs -> successfulJobs
namesapce -> namespace
ConfigMapResynce -> ConfigMapResync
2020-02-27 00:15:33 +09:00
sunxiaofei03
45d41ed9e5 replace iteration with hashmap in *state_of_world 2019-08-29 19:22:25 +08:00
David Xia
fabfd950b1
cleanup: fix some log and error capitalizations
Part of https://github.com/kubernetes/kubernetes/issues/15863
2019-07-20 18:26:16 -04:00
Kubernetes Prow Robot
5777fdfe31
Merge pull request #78105 from cwdsuzhou/narrow_down_lock
Narrow down the lock
2019-06-14 04:08:23 -07:00
caiweidong
e39ec09975 Add error info for plugin do not support attachment 2019-05-23 18:32:49 +08:00
caiweidong
5a0e7f19b6 Narrow down the lock 2019-05-20 19:12:27 +08:00
Jing Xu
7bac6ca73a Address comments
This commit addressed the comment and also add a unit test.
2019-01-11 10:57:37 -08:00
Jing Xu
562d0fea53 Handle failed attach operation leave uncertain volume attach state
This commit adds the unit tests for the PR. It also includes some files
that are affected by the function name changes.
2018-11-19 17:21:49 -08:00
Jing Xu
47331cf0a2 WIP: Handle failed attach operation leave uncertain volume attach state
This PR fixes issue #32727.

When an attach operation fails, it is still possible that the volume
will be attached to the node later. This PR adds the logic to record the
volume to node with attached state no matter whether the operation
succedded or not. If the operation fails, mark the attached state to
false. If the operation succeeded, mark the attached state to true. The
reconciler will still issue attach operation until it returns
successfully. If the pod is removed in the mean time, the reconciler
will issue detach operations for all the volumes no matter what is the
attached state.
2018-11-19 17:19:10 -08:00
Davanum Srinivas
954996e231
Move from glog to klog
- Move from the old github.com/golang/glog to k8s.io/klog
- klog as explicit InitFlags() so we add them as necessary
- we update the other repositories that we vendor that made a similar
change from glog to klog
  * github.com/kubernetes/repo-infra
  * k8s.io/gengo/
  * k8s.io/kube-openapi/
  * github.com/google/cadvisor
- Entirely remove all references to glog
- Fix some tests by explicit InitFlags in their init() methods

Change-Id: I92db545ff36fcec83afe98f550c9e630098b3135
2018-11-10 07:50:31 -05:00
houjun
9a84e413fc Fix missing 'break' 2018-10-23 17:14:05 +08:00
Jeff Grafton
23ceebac22 Run hack/update-bazel.sh 2018-06-22 16:22:57 -07:00
wackxu
b3ba80b223 update bazel 2018-02-27 20:23:36 +08:00
wackxu
f737ad62ed update import 2018-02-27 20:23:35 +08:00
Lei Wang
2e0abfa29f Fix grammar and log issue in volume cache code 2018-02-23 17:46:53 +08:00
Jeff Grafton
ef56a8d6bb Autogenerated: hack/update-bazel.sh 2018-02-16 13:43:01 -08:00
Di Xu
48388fec7e fix all the typos across the project 2018-02-11 11:04:14 +08:00
Kubernetes Submit Queue
7de1a8e0f5
Merge pull request #56288 from jsafrane/multiattach-pods
Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Add list of pods that use a volume to multiattach events

So users knows what pods are blocking a volume and can realize their error.

**Release note**:

```release-note
NONE
```

UX:
* User can get one of following events, depending what other pod(s) are already using a volume and in which namespace they are:
```
Multi-Attach error for volume"volume-name" Volume is already exclusively attached to one node and can't be attached to another
Multi-Attach error for volume "volume-name" Volume is already used by pod(s) pod3 and 1 pod(s) in different namespaces
```

* controller-manager gets always full logs:
  * When the node where is the volume attached is known:
        ```
        Multi-Attach error for volume "volume-name" (UniqueName: "fake-plugin/volume-name") from node "node1" Volume is already used by pods ns2/pod2, ns1/pod3 on node node2, node3
        ```

  * When the node where is the volume attached is not known:
        ```
        Multi-Attach error for volume "volume-name" (UniqueName: "fake-plugin/volume-name") from node "node1" Volume is already exclusively attached to node node2 and can't be attached to another
        ```

/kind bug
/sig storage
/assign @gnufied
2018-01-25 05:31:34 -08:00
Jan Safranek
e46c886bf3 Add list of pods that use a volume to multiattach events
So users knows what pods are blocking a volume and can realize their error.
2018-01-24 13:22:03 +01:00
Kubernetes Submit Queue
6f3e1dabe4
Merge pull request #57501 from linyouchong/linyouchong-20171221
Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

fix incorrect comment
2018-01-09 11:34:27 -08:00
Jeff Grafton
efee0704c6 Autogenerate BUILD files 2017-12-23 13:12:11 -08:00
linyouchong
4acc23b409 fix incorrect comment 2017-12-21 23:36:20 +08:00
guangxuli
cb73ab2b07 The printing level for node updated failed info should be used WARNING type
just use Warning instead of Warningf
2017-11-01 16:54:49 +08:00
Hemant Kumar
e3f0c8bb2d Fixes spam from node status updates
The same error is logged in 2 places which is unncessary.
2017-10-19 09:37:07 -04:00
Kubernetes Submit Queue
1d8f1e268f Merge pull request #47699 from supereagle/fix-typos
Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

fix typos: remove duplicated word in comments

**What this PR does / why we need it**: Remove the duplicated word `the` in comments

**Which issue this PR fixes** : fixes #

**Special notes for your reviewer**:

```release-note
NONE
```
2017-10-17 02:35:52 -07:00
Jeff Grafton
aee5f457db update BUILD files 2017-10-15 18:18:13 -07:00
Hemant Kumar
414c3104ca Make sure we use rwlocks not just RLock 2017-10-10 17:52:55 -04:00
Hemant Kumar
67d4c40849 Fix spam of multiattach errors in event logs
We should be careful while generating multiattach errors.
We seem to be generating too many of them because old code
had minor bug.
2017-10-03 15:45:06 -04:00
supereagle
87c29a08e1 fix typos: remove duplicated word in comments 2017-09-16 14:38:10 +08:00
Hemant Kumar
8edae9b3fc Always populate volume status from node 2017-09-12 09:03:42 -04:00
Cheng Xing
1234d2f500 On AttachDetachController node status update, do not retry when node doesn't exist but keep the node entry in cache 2017-08-16 15:42:15 -07:00
Jeff Grafton
a7f49c906d Use buildozer to delete licenses() rules except under third_party/ 2017-08-11 09:32:39 -07:00
Jeff Grafton
33276f06be Use buildozer to remove deprecated automanaged tags 2017-08-11 09:31:50 -07:00
Chao Xu
60604f8818 run hack/update-all 2017-06-22 11:31:03 -07:00
Chao Xu
f4989a45a5 run root-rewrite-v1-..., compile 2017-06-22 10:25:57 -07:00
Kubernetes Submit Queue
b795ec7de0 Merge pull request #42251 from justinsb/simplify_append
Automatic merge from submit-queue (batch tested with PRs 42252, 42251, 42249, 47512, 47887)

volumes: simplify append-to-slice code

Minor simplification - can append to empty/nil slice.

Part of #40583

```release-note
NONE
```
2017-06-21 22:13:27 -07:00