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

Use workqueue model in expand controller #75386

Open
wants to merge 3 commits into
base: master
from

Conversation

@wongma7
Copy link
Contributor

wongma7 commented Mar 14, 2019

What type of PR is this?
/kind cleanup

What this PR does / why we need it:

  • potentially improve performance (no need to populate all PVCs into memory every 2mins)
  • resize PVCs as they come off the queue instead of every 400ms
  • better maintainability: less code & structs to test/worry about, follows established controller pattern
  • see also #71760

Which issue(s) this PR fixes:

Fixes #71760

Special notes for your reviewer: the workqueue basically replaces operationexecutor's nestedPendingOperations.

Does this PR introduce a user-facing change?:

NONE
@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

k8s-ci-robot commented Mar 18, 2019

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: wongma7
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: lavalamp

If they are not already assigned, you can assign the PR to them by writing /assign @lavalamp in a comment when ready.

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

@wongma7 wongma7 force-pushed the wongma7:expand-workqueue branch from d6c8dcf to 8fb0092 Mar 18, 2019

@wongma7 wongma7 changed the title WIP: Use workqueue model in expand controller Use workqueue model in expand controller Mar 18, 2019

@wongma7 wongma7 force-pushed the wongma7:expand-workqueue branch from 8fb0092 to 8bc5abe Mar 18, 2019

@fejta-bot

This comment has been minimized.

Copy link

fejta-bot commented Mar 18, 2019

This PR may require API review.

If so, when the changes are ready, complete the pre-review checklist and request an API review.

Status of requested reviews is tracked in the API Review project.

@caesarxuchao

This comment has been minimized.

Copy link
Member

caesarxuchao commented Mar 18, 2019

/assign @cheftako

@gnufied

This comment has been minimized.

Copy link
Member

gnufied commented Mar 18, 2019

/assign

Show resolved Hide resolved pkg/controller/volume/expand/expand_controller.go
Show resolved Hide resolved pkg/controller/volume/expand/expand_controller.go Outdated
Show resolved Hide resolved pkg/volume/util/resize_util.go
expc.enqueuePVC(new)
}
},
DeleteFunc: expc.enqueuePVC,

This comment has been minimized.

@gnufied

gnufied Mar 18, 2019

Member

Do we need to forget the key PVC was deleted?

This comment has been minimized.

@wongma7

wongma7 Mar 19, 2019

Author Contributor

It will be forgotten if it DNE in the informer

Show resolved Hide resolved pkg/controller/apis/config/v1alpha1/defaults.go Outdated

@wongma7 wongma7 force-pushed the wongma7:expand-workqueue branch from 718baa2 to 5ff4944 Mar 19, 2019

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

k8s-ci-robot commented Mar 24, 2019

@wongma7: PR needs rebase.

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/test-infra repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.