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

Add plugin watcher design #2369

Merged
merged 1 commit into from
Aug 22, 2018
Merged

Add plugin watcher design #2369

merged 1 commit into from
Aug 22, 2018

Conversation

vikaschoudhary16
Copy link
Contributor

@vikaschoudhary16 vikaschoudhary16 commented Jul 12, 2018 •
edited
Loading

This PR is for adding design of an already merged feature. Original design was on gdoc. With some updates adding the same here for better tracking.

/cc @tengqm @derekwaynecarr @vishh @jiayingz @ConnorDoyle @dchen1107 @RenaudWasTaken @sjenning

@k8s-ci-robot k8s-ci-robot requested a review from ConnorDoyle July 12, 2018 11:16
@k8s-ci-robot k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jul 12, 2018
@k8s-ci-robot k8s-ci-robot added kind/design Categorizes issue or PR as related to design. sig/node Categorizes an issue or PR as relevant to SIG Node. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Jul 12, 2018
@vikaschoudhary16 vikaschoudhary16 mentioned this pull request Jul 12, 2018
Closed
20 tasks
@jdumars
Copy link
Member

jdumars commented Jul 13, 2018

Is this a KEP?

@vikaschoudhary16
Copy link
Contributor Author

@jdumars This is not in KEP format. Is it required to convert it into KEP?

@jdumars
Copy link
Member

jdumars commented Jul 17, 2018 •
edited
Loading

@vikaschoudhary16 I don't think it's required to be a KEP per se, but I'm not sure why it isn't since the proposal process is being deprecated in favor of KEPs. I'd also like to insert it into the KEP tracking process here: https://github.com/kubernetes-sigs/architecture-tracking/projects/2

@vikaschoudhary16 vikaschoudhary16 mentioned this pull request Aug 1, 2018
Closed
@saad-ali
Copy link
Member

saad-ali commented Aug 2, 2018 •
edited
Loading

I don't think this needs to be tracked as a KEP since the feature has already been implemented (this is adding the design documentation).

@vikaschoudhary16 is there any difference between what is documented here and what is implemented? If not, LGTM

@vikaschoudhary16
Copy link
Contributor Author

@saad-ali There is no difference with implementation.

ScorpioCPH
ScorpioCPH reviewed Aug 3, 2018
View reviewed changes
contributors/design-proposals/node/plugin-watcher.md
// This allows the plugin to register using one endpoint and possibly use
// a different socket for control operations. CSI uses this model to delegate
// its registration external from the plugin.
string endpoint = 3;
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: these lines are out of alignment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ScorpioCPH not sure, why it is showing dis-alignment here. If you click "view", it shows correct and also if you pull this PR, on local machine also it shows correct. :)

@saad-ali
Copy link
Member

saad-ali commented Aug 3, 2018

Address nits, otherwise:

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 3, 2018
@k8s-ci-robot k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. area/contributor-guide Issues or PRs related to the contributor guide area/github-management Issues or PRs related to GitHub Management subproject committee/steering Denotes an issue or PR intended to be handled by the steering committee. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. and removed lgtm "Looks good to me", indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Aug 21, 2018
@k8s-ci-robot k8s-ci-robot added sig/scheduling Categorizes an issue or PR as relevant to SIG Scheduling. sig/service-catalog Categorizes an issue or PR as relevant to SIG Service Catalog. sig/storage Categorizes an issue or PR as relevant to SIG Storage. sig/testing Categorizes an issue or PR as relevant to SIG Testing. sig/ui Categorizes an issue or PR as relevant to SIG UI. area/provider/vmware Issues or PRs related to vmware provider sig/windows Categorizes an issue or PR as relevant to SIG Windows. wg/app-def wg/cluster-api Categorizes an issue or PR as relevant to wg-cluster-api. wg/iot-edge Categorizes an issue or PR as relevant to WG IOT Edge. wg/multitenancy Categorizes an issue or PR as relevant to WG Multitenancy. wg/policy Categorizes an issue or PR as relevant to WG Policy. labels Aug 21, 2018
@vikaschoudhary16 vikaschoudhary16 force-pushed the plugin-watcher branch 2 times, most recently from 309dfe9 to 56fe07d Compare August 21, 2018 05:48
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Aug 21, 2018
@saad-ali
Copy link
Member

This has already been implemented in the last quarter.

I'm ok with merge.

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 21, 2018
@derekwaynecarr
Copy link
Member

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: derekwaynecarr, saad-ali

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 22, 2018
@k8s-ci-robot k8s-ci-robot merged commit 121b63d into kubernetes:master Aug 22, 2018
@zshi-redhat zshi-redhat mentioned this pull request Jan 21, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ScorpioCPH ScorpioCPH ScorpioCPH left review comments

@ConnorDoyle ConnorDoyle Awaiting requested review from ConnorDoyle

@dchen1107 dchen1107 Awaiting requested review from dchen1107

@derekwaynecarr derekwaynecarr Awaiting requested review from derekwaynecarr

@jiayingz jiayingz Awaiting requested review from jiayingz

@RenaudWasTaken RenaudWasTaken Awaiting requested review from RenaudWasTaken

@tengqm tengqm Awaiting requested review from tengqm

@vishh vishh Awaiting requested review from vishh

Assignees

@saad-ali saad-ali

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/contributor-guide Issues or PRs related to the contributor guide area/github-management Issues or PRs related to GitHub Management subproject area/provider/azure Issues or PRs related to azure provider area/provider/ibmcloud Issues or PRs related to ibmcloud provider area/provider/openstack Issues or PRs related to openstack provider area/provider/vmware Issues or PRs related to vmware provider cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. committee/steering Denotes an issue or PR intended to be handled by the steering committee. kind/design Categorizes issue or PR as related to design. lgtm "Looks good to me", indicates that a PR is ready to be merged. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. sig/apps Categorizes an issue or PR as relevant to SIG Apps. sig/architecture Categorizes an issue or PR as relevant to SIG Architecture. sig/auth Categorizes an issue or PR as relevant to SIG Auth. sig/autoscaling Categorizes an issue or PR as relevant to SIG Autoscaling. sig/cli Categorizes an issue or PR as relevant to SIG CLI. sig/cloud-provider Categorizes an issue or PR as relevant to SIG Cloud Provider. sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. sig/contributor-experience Categorizes an issue or PR as relevant to SIG Contributor Experience. sig/docs Categorizes an issue or PR as relevant to SIG Docs. sig/instrumentation Categorizes an issue or PR as relevant to SIG Instrumentation. sig/multicluster Categorizes an issue or PR as relevant to SIG Multicluster. sig/network Categorizes an issue or PR as relevant to SIG Network. sig/node Categorizes an issue or PR as relevant to SIG Node. sig/release Categorizes an issue or PR as relevant to SIG Release. sig/scalability Categorizes an issue or PR as relevant to SIG Scalability. sig/scheduling Categorizes an issue or PR as relevant to SIG Scheduling. sig/service-catalog Categorizes an issue or PR as relevant to SIG Service Catalog. sig/storage Categorizes an issue or PR as relevant to SIG Storage. sig/testing Categorizes an issue or PR as relevant to SIG Testing. sig/ui Categorizes an issue or PR as relevant to SIG UI. sig/windows Categorizes an issue or PR as relevant to SIG Windows. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. ug/big-data Categorizes an issue or PR as relevant to ug-big-data. wg/cluster-api Categorizes an issue or PR as relevant to wg-cluster-api. wg/iot-edge Categorizes an issue or PR as relevant to WG IOT Edge. wg/multitenancy Categorizes an issue or PR as relevant to WG Multitenancy. wg/policy Categorizes an issue or PR as relevant to WG Policy.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@vikaschoudhary16 @jdumars @saad-ali @derekwaynecarr @k8s-ci-robot @ScorpioCPH