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

feat: add outOfTree plugin entry when initializing scheduler #1663

Merged

Conversation

kerthcet
Copy link
Member

@kerthcet kerthcet commented Apr 26, 2022

Signed-off-by: kerthcet kerthcet@gmail.com

What type of PR is this?
/kind feature

What this PR does / why we need it:

Which issue(s) this PR fixes:
Fixes #1371

Special notes for your reviewer:
Now we can initialize OutOfTree Plugin like below, then we can compile customized karmada scheduler.

command := app.NewSchedulerCommand(stopChan,
    app.WithPlugin(apiinstalled.Name, apiinstalled.New),
)

Does this PR introduce a user-facing change?:

`karmada-scheduler`: People are now able to extend plugins in an out-of-tree mode.

@karmada-bot karmada-bot added the kind/feature Categorizes issue or PR as related to a new feature. label Apr 26, 2022
@karmada-bot karmada-bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Apr 26, 2022
@kerthcet
Copy link
Member Author

I was try to add e2e test, but didn't find any place to change the configuration in initializing scheduler.

@kerthcet
Copy link
Member Author

cc @huone1

@kerthcet kerthcet force-pushed the feature/add-outoftree-registry branch from 9ad1f17 to 2e35f2b Compare April 26, 2022 10:03
@karmada-bot karmada-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Apr 26, 2022
Signed-off-by: kerthcet <kerthcet@gmail.com>
@kerthcet kerthcet force-pushed the feature/add-outoftree-registry branch from 2e35f2b to 84cc6f8 Compare June 1, 2022 12:33
@karmada-bot karmada-bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jun 1, 2022
@kerthcet
Copy link
Member Author

kerthcet commented Jun 1, 2022

I was try to add e2e test, but didn't find any place to change the configuration in initializing scheduler.

Sorry for the long delayed response, out of bandwidth in the past weeks. Can you help me with this @huone1 , I would like to add some integration tests or e2e tests. But have no idea how to initialize scheduler in karmada's test framework.

@RainbowMango
Copy link
Member

@prodanlabs Is this extension way you are looking for?

@RainbowMango
Copy link
Member

@kerthcet Sorry for let this sit, I'll look at it and get back to you(probably next week.)

@kerthcet
Copy link
Member Author

kerthcet commented Jun 2, 2022

@prodanlabs Is this extension way you are looking for?

FYI, it we merged this, we can extend out-of-tree plugin like below when instantiating the scheduler. Foo is the plugin we'd like to apply.

command := app.NewSchedulerCommand(stopChan,
    app.WithPlugin(Foo.Name, Foo.New),
)

@prodanlabs
Copy link
Member

This function is what we need.

Every time you upgrade karmada, you need to recompile karmada-scheduler. It would be better if it can support external expansion. Of course, this can be discussed separately.

@prodanlabs
Copy link
Member

@prodanlabs Is this extension way you are looking for?

FYI, it we merged this, we can extend out-of-tree plugin like below when instantiating the scheduler. Foo is the plugin we'd like to apply.

command := app.NewSchedulerCommand(stopChan,
    app.WithPlugin(Foo.Name, Foo.New),
)

Hi @kerthcet ,still need to hardcode, right?

@kerthcet
Copy link
Member Author

@prodanlabs Is this extension way you are looking for?

FYI, it we merged this, we can extend out-of-tree plugin like below when instantiating the scheduler. Foo is the plugin we'd like to apply.

command := app.NewSchedulerCommand(stopChan,
    app.WithPlugin(Foo.Name, Foo.New),
)

Hi @kerthcet ,still need to hardcode, right?

Yes, you have to compile yourself. Here's another way the kubernetes community is looking for, but still exploring. kubernetes/kubernetes#100723

@prodanlabs
Copy link
Member

@prodanlabs Is this extension way you are looking for?

FYI, it we merged this, we can extend out-of-tree plugin like below when instantiating the scheduler. Foo is the plugin we'd like to apply.

command := app.NewSchedulerCommand(stopChan,
    app.WithPlugin(Foo.Name, Foo.New),
)

Hi @kerthcet ,still need to hardcode, right?

Yes, you have to compile yourself. Here's another way the kubernetes community is looking for, but still exploring. kubernetes/kubernetes#100723

Thanks.

@RainbowMango
Copy link
Member

Thanks @kerthcet and sorry again for letting this sit so long.

/lgtm
/approve

@karmada-bot karmada-bot added the lgtm Indicates that a PR is ready to be merged. label Jun 10, 2022
@karmada-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: RainbowMango

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

@karmada-bot karmada-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 10, 2022
@karmada-bot karmada-bot merged commit e68c089 into karmada-io:master Jun 10, 2022
@RainbowMango
Copy link
Member

Note: I added a release note to the PR description, this is a significant feature that should be present in the next releases.

@kerthcet kerthcet deleted the feature/add-outoftree-registry branch June 10, 2022 07:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. kind/feature Categorizes issue or PR as related to a new feature. lgtm Indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Extension for out-of-tree plugins
5 participants