api: define API for MultiClusterService #4290

jwcesign · 2023-11-21T07:53:12Z

What type of PR is this?
/kind api-change

What this PR does / why we need it:
Referring to #4287

Which issue(s) this PR fixes:
Part of #4292

Special notes for your reviewer:
none

Does this PR introduce a user-facing change?:

`API`: Introduced `ServiceProvisionClusters` and `ServiceConsumptionClusters` to `MultiClusterService`, which will be used to specify service source and consumption place.

codecov-commenter · 2023-11-21T09:24:14Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (5b8a7f7) 52.00% compared to head (a39416c) 52.01%.
Report is 4 commits behind head on master.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #4290      +/-   ##
==========================================
+ Coverage   52.00%   52.01%   +0.01%     
==========================================
  Files         242      242              
  Lines       23993    23984       -9     
==========================================
- Hits        12478    12476       -2     
+ Misses      10834    10828       -6     
+ Partials      681      680       -1

Flag	Coverage Δ
unittests	`52.01% <100.00%> (+0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

jwcesign · 2023-11-21T10:14:21Z

/cc @XiShanYongYe-Chang @chaunceyjiang

chaunceyjiang · 2023-11-21T11:12:02Z

pkg/apis/networking/v1alpha1/service_types.go

 	// +optional
-	Range ExposureRange `json:"range,omitempty"`
+	DiscoveryStrategy ServiceDiscoveryStrategy `json:"discoveryStrategy,omitempty"`


In the proposal of MCS, there is no concept like DiscoveryStrategy.

This Range is used for the propagation range of ServiceImport, it has nothing to do with EndpointSlice. So what's your reason for deleting this field?

https://github.com/karmada-io/karmada/blob/master/docs/proposals/service-discovery/README.md There is annotation: discovery.karmada.io/strategy, I move it to the spec

This field is not used, @XiShanYongYe-Chang told me. If we want to utilize the ServiceImport/ServiceExport in the future, we can use the same fields. Propagate ServiceImport to ClientLocations, ServiceExport to ServerLocations

We used this field in another proposal and it needs to be synchronized there after the current proposal is merged.

@chaunceyjiang Are you suggesting not introducing this field this time as it hasn't been designed yet, and we can take the default behavior( that is RemoteAndLocal) during implementation?

I agree with it.

Are you suggesting not introducing this field this time as it hasn't been designed yet, and we can take the default behavior( that is RemoteAndLocal) during implementation?

+1

Why not remove it this time?

cc @chaunceyjiang for conforming

pkg/apis/networking/v1alpha1/service_types.go

RainbowMango · 2023-11-23T08:50:06Z

Just a reminder, the chart lint is failing.

kevin-wangzefeng

Some other options for the field names:
ServerSideLocations vs. ClientSideLocations
ProvisionedByClusters vs. ConsumedByClusters

pkg/apis/networking/v1alpha1/service_types.go

RainbowMango · 2023-11-24T03:37:55Z

Echo from the workflow:
https://github.com/karmada-io/karmada/actions/runs/6971498068/job/18971689975?pr=4290

========================================================================================================================
Deleting release "karmada-0vspli2tsv"...
>>> helm uninstall karmada-0vspli2tsv --namespace karmada-0vspli2tsv --timeout 800s
Error: uninstallation completed with 1 error(s): timed out waiting for the condition
Error deleting Helm release: failed waiting for process: exit status 1
Deleting namespace "karmada-0vspli2tsv"...

Seems timeout when uninstall Helm release. Let's give it another try.

jwcesign · 2023-11-24T03:47:01Z

Seems timeout when uninstall Helm release. Let's give it another try.

I tried it in my local env, it works fine.
helm installation needs image cfssl/cfssl:latest, 1.31GiB, I think we better load it to kind first before running helm install to save time.

Signed-off-by: jwcesign <[email protected]>

RainbowMango · 2023-11-24T06:12:22Z

helm installation needs image cfssl/cfssl:latest, 1.31GiB, I think we better load it to kind first before running helm install to save time.

Is this the main reason?
See this log:
https://github.com/karmada-io/karmada/actions/runs/6971498068/job/18971689975?pr=4290#step:11:55

Successfully pulled image "docker.io/cfssl/cfssl:latest" in 11.903950394s (11.903965362s including waiting)

It only takes 11s to pull the cfssl.

pkg/webhook/multiclusterservice/validating.go

pkg/webhook/multiclusterservice/validating_test.go

RainbowMango

/lgtm
/approve

Please cc me if unrelated tests fail.

karmada-bot · 2023-11-24T06:39:15Z

[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:

~~OWNERS~~ [RainbowMango]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

karmada-bot added the kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API label Nov 21, 2023

karmada-bot requested review from pidb and Poor12 November 21, 2023 07:53

karmada-bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Nov 21, 2023

jwcesign force-pushed the mcs-api branch 2 times, most recently from 5d3498f to bdaffc7 Compare November 21, 2023 08:47

jwcesign mentioned this pull request Nov 21, 2023

[umbrella] Add Implementation for Service discovery with native Kubernetes naming and resolution #4292

Open

25 tasks

jwcesign force-pushed the mcs-api branch from bdaffc7 to 89632da Compare November 21, 2023 09:12

karmada-bot requested review from chaunceyjiang and XiShanYongYe-Chang November 21, 2023 10:14

chaunceyjiang reviewed Nov 21, 2023

View reviewed changes

RainbowMango reviewed Nov 22, 2023

View reviewed changes

pkg/apis/networking/v1alpha1/service_types.go Show resolved Hide resolved

RainbowMango reviewed Nov 22, 2023

View reviewed changes

pkg/apis/networking/v1alpha1/service_types.go Outdated Show resolved Hide resolved

RainbowMango reviewed Nov 22, 2023

View reviewed changes

pkg/apis/networking/v1alpha1/service_types.go Outdated Show resolved Hide resolved

XiShanYongYe-Chang reviewed Nov 23, 2023

View reviewed changes

pkg/apis/networking/v1alpha1/service_types.go Outdated Show resolved Hide resolved

jwcesign force-pushed the mcs-api branch 4 times, most recently from eff274f to 6b73315 Compare November 23, 2023 06:13

kevin-wangzefeng reviewed Nov 23, 2023

View reviewed changes

pkg/apis/networking/v1alpha1/service_types.go Outdated Show resolved Hide resolved

pkg/apis/networking/v1alpha1/service_types.go Outdated Show resolved Hide resolved

jwcesign force-pushed the mcs-api branch from 6b73315 to a60ab64 Compare November 23, 2023 12:48

RainbowMango reviewed Nov 23, 2023

View reviewed changes

pkg/apis/networking/v1alpha1/service_types.go Outdated Show resolved Hide resolved

jwcesign force-pushed the mcs-api branch 3 times, most recently from 0f36fc1 to 23abfd4 Compare November 23, 2023 15:12

api: define API for MultiClusterService

a39416c

Signed-off-by: jwcesign <[email protected]>

RainbowMango reviewed Nov 24, 2023

View reviewed changes

pkg/webhook/multiclusterservice/validating.go Outdated Show resolved Hide resolved

RainbowMango reviewed Nov 24, 2023

View reviewed changes

pkg/webhook/multiclusterservice/validating.go Outdated Show resolved Hide resolved

RainbowMango reviewed Nov 24, 2023

View reviewed changes

pkg/webhook/multiclusterservice/validating_test.go Outdated Show resolved Hide resolved

jwcesign force-pushed the mcs-api branch from 23abfd4 to a39416c Compare November 24, 2023 06:34

RainbowMango approved these changes Nov 24, 2023

View reviewed changes

karmada-bot assigned RainbowMango Nov 24, 2023

karmada-bot added the lgtm Indicates that a PR is ready to be merged. label Nov 24, 2023

karmada-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 24, 2023

karmada-bot merged commit 1d4a95c into karmada-io:master Nov 24, 2023
13 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

api: define API for MultiClusterService #4290

api: define API for MultiClusterService #4290

jwcesign commented Nov 21, 2023 •

edited by RainbowMango

Loading

codecov-commenter commented Nov 21, 2023 •

edited

Loading

jwcesign commented Nov 21, 2023

chaunceyjiang Nov 21, 2023 •

edited

Loading

jwcesign Nov 21, 2023 •

edited

Loading

XiShanYongYe-Chang Nov 21, 2023

RainbowMango Nov 22, 2023

jwcesign Nov 22, 2023

RainbowMango Nov 23, 2023

jwcesign Nov 23, 2023

RainbowMango commented Nov 23, 2023

kevin-wangzefeng left a comment

RainbowMango commented Nov 24, 2023

jwcesign commented Nov 24, 2023

RainbowMango commented Nov 24, 2023

RainbowMango left a comment

karmada-bot commented Nov 24, 2023

api: define API for MultiClusterService #4290

api: define API for MultiClusterService #4290

Conversation

jwcesign commented Nov 21, 2023 • edited by RainbowMango Loading

codecov-commenter commented Nov 21, 2023 • edited Loading

Codecov Report

jwcesign commented Nov 21, 2023

chaunceyjiang Nov 21, 2023 • edited Loading

Choose a reason for hiding this comment

jwcesign Nov 21, 2023 • edited Loading

Choose a reason for hiding this comment

XiShanYongYe-Chang Nov 21, 2023

Choose a reason for hiding this comment

RainbowMango Nov 22, 2023

Choose a reason for hiding this comment

jwcesign Nov 22, 2023

Choose a reason for hiding this comment

RainbowMango Nov 23, 2023

Choose a reason for hiding this comment

jwcesign Nov 23, 2023

Choose a reason for hiding this comment

RainbowMango commented Nov 23, 2023

kevin-wangzefeng left a comment

Choose a reason for hiding this comment

RainbowMango commented Nov 24, 2023

jwcesign commented Nov 24, 2023

RainbowMango commented Nov 24, 2023

RainbowMango left a comment

Choose a reason for hiding this comment

karmada-bot commented Nov 24, 2023

jwcesign commented Nov 21, 2023 •

edited by RainbowMango

Loading

codecov-commenter commented Nov 21, 2023 •

edited

Loading

chaunceyjiang Nov 21, 2023 •

edited

Loading

jwcesign Nov 21, 2023 •

edited

Loading