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

Remove definitly all remaining usage of context.TODO() #8701

Closed
leseb opened this issue Sep 13, 2021 · 8 comments · Fixed by #9685
Closed

Remove definitly all remaining usage of context.TODO() #8701

leseb opened this issue Sep 13, 2021 · 8 comments · Fixed by #9685
Assignees
@weirdwiz
Labels
feature keepalive
Projects
v1.9

Comments

@leseb
Copy link
Member

leseb commented Sep 13, 2021 •
edited

This is a follow-up from #8607. Each function using context.TODO() internally must instead ask for a context. Top-level callers now have a context.Context available so let's use it.

This excludes the tests package directory for now.
@leseb leseb added this to To do in v1.8 via automation Sep 13, 2021
y1r added a commit to y1r/rook that referenced this issue Oct 23, 2021
@y1r
ceph: add context parameter to opcontroller
978d8a5 
This commit adds context parameter to utilities in opcontroller to
remove context.TODO use in opcontroller. By this, we can handle
cancellation cancellation of reconcilers in a fine-grained way.
Part of rook#8701.

Signed-off-by: Yuichiro Ueno <y1r.ueno@gmail.com>
y1r added a commit to y1r/rook that referenced this issue Oct 23, 2021
@y1r
ceph: add context parameter to opcontroller
7232f2d 
This commit adds context parameter to utilities in opcontroller to
remove context.TODO use in opcontroller. By this, we can handle
cancellation cancellation of reconcilers in a fine-grained way.
Part of rook#8701.

Signed-off-by: Yuichiro Ueno <y1r.ueno@gmail.com>
y1r added a commit to y1r/rook that referenced this issue Oct 23, 2021
@y1r
ceph: add context parameter to opcontroller
a38c1f1 
This commit adds context parameter to utilities in opcontroller to
remove context.TODO use in opcontroller. By this, we can handle
cancellation of reconcilers in a fine-grained way. Part of rook#8701.

Signed-off-by: Yuichiro Ueno <y1r.ueno@gmail.com>
@y1r y1r mentioned this issue Oct 23, 2021
Merged
10 tasks
@leseb
Copy link
Member Author

leseb commented Oct 25, 2021

@y1r are you planning on finishing this work? Thanks and appreciate your initial contribution.

@y1r
Copy link
Contributor

y1r commented Oct 25, 2021

@leseb Yes! I want to try this issue. It seems to be a good chance to learn Rook codebase.

@parth-gr
Copy link
Member

parth-gr commented Oct 26, 2021 •
edited

@y1r also please look into this issue, it's equivalent to what you're doing, but it's compact just to a K8util package.
So mentioning you for the same as it will be covered with this issue.
Thanks!

@leseb
Copy link
Member Author

leseb commented Dec 13, 2021

Closed in #9023

@leseb leseb closed this as completed Dec 13, 2021
v1.8 automation moved this from To do to Done Dec 13, 2021
@leseb
Copy link
Member Author

leseb commented Dec 13, 2021

We still have some.

@leseb leseb reopened this Dec 13, 2021
v1.8 automation moved this from Done to In progress Dec 13, 2021
@weirdwiz
Copy link
Contributor

I would like to take this up, if it's not been worked upon by someone else.

@subhamkrai
Copy link
Contributor

I would like to take this up, if it's not been worked upon by someone else.

@weirdwiz it's all yours now.

weirdwiz added a commit to weirdwiz/rook that referenced this issue Jan 31, 2022
@weirdwiz
core: Add context parameter for updating device configmap
eb8d42b 
This commit adds context parameter to the updateDeviceCM() function and other top
level calling functions. This will allow us to handle cancellations
while talking to the k8s API.

Closes: rook#8701
Signed-off-by: Divyansh Kamboj <dkamboj@redhat.com>
weirdwiz added a commit to weirdwiz/rook that referenced this issue Jan 31, 2022
@weirdwiz
core: Add context parameter to AdminClusterInfo
b4d9cce 
This commit removes the usage of context.TODO while creating ClusterInfo
to access the cluster as an admin. This commit also adds the context
parameter to the functions calling AdminClusterInfo to pass down the
context. This will allow us to handle cancellation of all the methods
being using the context from ClusterInfo.

Closes: rook#8701
Signed-off-by: Divyansh Kamboj <dkamboj@redhat.com>
weirdwiz added a commit to weirdwiz/rook that referenced this issue Jan 31, 2022
@weirdwiz
core: add context parameter in functions executing commands in container
1f63a51 
This commit adds context parameter to function
ExecCommandInContainerWithFullOutPut(). The context parameter is used
while getting the list of pods.

Closes: rook#8701
Signed-off-by: Divyansh Kamboj <dkamboj@redhat.com>
weirdwiz added a commit to weirdwiz/rook that referenced this issue Jan 31, 2022
@weirdwiz
osd: add context parameter to configTLS
a4102fa 
This commit removes context.TODO and adds context parameter to transfer
the context down from calling functions.

This commit also renames contex, of type ClusterdContext, to
clusterdContext in kms_test.go, due to conflicts in naming while adding
context.Context parameter to vaultClient function in the tests.

Closes: rook#8701
Signed-off-by: Divyansh Kamboj <dkamboj@redhat.com>
@weirdwiz weirdwiz mentioned this issue Feb 1, 2022
Merged
10 tasks
weirdwiz added a commit to weirdwiz/rook that referenced this issue Feb 2, 2022
@weirdwiz
core: add context parameter for updating device configmap
8f9875d 
This commit adds context parameter to the updateDeviceCM() function and other top
level calling functions. This will allow us to handle cancellations
while talking to the k8s API.

Closes: rook#8701
Signed-off-by: Divyansh Kamboj <dkamboj@redhat.com>
weirdwiz added a commit to weirdwiz/rook that referenced this issue Feb 2, 2022
@weirdwiz
core: add context parameter to AdminClusterInfo
b407616 
This commit removes the usage of context.TODO while creating ClusterInfo
to access the cluster as an admin. This commit also adds the context
parameter to the functions calling AdminClusterInfo to pass down the
context. This will allow us to handle cancellation of all the methods
being using the context from ClusterInfo.

Closes: rook#8701
Signed-off-by: Divyansh Kamboj <dkamboj@redhat.com>
weirdwiz added a commit to weirdwiz/rook that referenced this issue Feb 2, 2022
@weirdwiz
core: add context parameter in functions executing commands in container
01af385 
This commit adds context parameter to function
ExecCommandInContainerWithFullOutPut(). The context parameter is used
while getting the list of pods.

Closes: rook#8701
Signed-off-by: Divyansh Kamboj <dkamboj@redhat.com>
weirdwiz added a commit to weirdwiz/rook that referenced this issue Feb 2, 2022
@weirdwiz
osd: add context parameter to configTLS
12509e3 
This commit removes context.TODO and adds context parameter to transfer
the context down from calling functions.

This commit also renames contex, of type ClusterdContext, to
clusterdContext in kms_test.go, due to conflicts in naming while adding
context.Context parameter to vaultClient function in the tests.

Closes: rook#8701
Signed-off-by: Divyansh Kamboj <dkamboj@redhat.com>
weirdwiz added a commit to weirdwiz/rook that referenced this issue Feb 7, 2022
@weirdwiz
core: add context parameter to functions
6afa6b2 
This commit adds context parameter to various functions, and remove the
usage of context.TODO.

Closes: rook#8701
Signed-off-by: Divyansh Kamboj <dkamboj@redhat.com>
@travisn travisn removed this from In progress in v1.8 Feb 8, 2022
@jbw976 jbw976 added this to To do in v1.9 Feb 8, 2022
@jbw976 jbw976 moved this from To do to In progress in v1.9 Feb 8, 2022
@jbw976 jbw976 moved this from In progress to Review in Progress in v1.9 Feb 8, 2022
weirdwiz added a commit to weirdwiz/rook that referenced this issue Feb 9, 2022
@weirdwiz
core: add context parameter to functions
c363c6e 
This commit adds context parameter to various functions, and remove the
usage of context.TODO.

Closes: rook#8701
Signed-off-by: Divyansh Kamboj <dkamboj@redhat.com>
weirdwiz added a commit to weirdwiz/rook that referenced this issue Feb 21, 2022
@weirdwiz
core: add context parameter to functions
01b452e 
This commit adds context parameter to various functions, and remove the
usage of context.TODO.

Closes: rook#8701
Signed-off-by: Divyansh Kamboj <dkamboj@redhat.com>
weirdwiz added a commit to weirdwiz/rook that referenced this issue Mar 21, 2022
@weirdwiz
core: add context parameter to functions
b5b6290 
This commit adds context parameter to various functions, and remove the
usage of context.TODO.

Closes: rook#8701
Signed-off-by: Divyansh Kamboj <dkamboj@redhat.com>
weirdwiz added a commit to weirdwiz/rook that referenced this issue Mar 21, 2022
@weirdwiz
core: add context parameter to functions
ae6b4d1 
This commit adds context parameter to various functions, and remove the
usage of context.TODO.

Closes: rook#8701
Signed-off-by: Divyansh Kamboj <dkamboj@redhat.com>
weirdwiz added a commit to weirdwiz/rook that referenced this issue Mar 22, 2022
@weirdwiz
core: add context parameter to functions
9008409 
This commit adds context parameter to various functions, and remove the
usage of context.TODO.

Closes: rook#8701
Signed-off-by: Divyansh Kamboj <dkamboj@redhat.com>
@github-actions
Copy link

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in a week if no further activity occurs. Thank you for your contributions.

@leseb leseb added keepalive and removed wontfix labels Mar 29, 2022
@travisn travisn moved this from Review in Progress to Done in v1.9 Apr 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@weirdwiz weirdwiz

Labels
feature keepalive
Projects
No open projects
v1.9
Done
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Remove context.TODO and add context parameter weirdwiz/rook
5 participants
@leseb @y1r @weirdwiz @subhamkrai @parth-gr