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

add MultiClusterService API #3644

Merged

Conversation

XiShanYongYe-Chang
Copy link
Member

What type of PR is this?

/kind api-change

What this PR does / why we need it:

add MultiClusterService API

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

NONE(part of the feature)

@karmada-bot karmada-bot added the kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API label Jun 6, 2023
@karmada-bot karmada-bot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Jun 6, 2023
@XiShanYongYe-Chang XiShanYongYe-Chang force-pushed the add-MultiClusterService-api branch 3 times, most recently from 8dc6b28 to db3eb63 Compare June 7, 2023 01:55
@XiShanYongYe-Chang XiShanYongYe-Chang marked this pull request as draft June 8, 2023 03:00
@karmada-bot karmada-bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 8, 2023
@XiShanYongYe-Chang XiShanYongYe-Chang force-pushed the add-MultiClusterService-api branch 2 times, most recently from fdc0c27 to 7385ee2 Compare June 20, 2023 11:31
@XiShanYongYe-Chang XiShanYongYe-Chang marked this pull request as ready for review June 20, 2023 11:31
@karmada-bot karmada-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 20, 2023
Copy link
Member

@RainbowMango RainbowMango left a comment

Choose a reason for hiding this comment

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

Please look at the alternative:

// MultiClusterService is a named abstraction of multi-cluster software service.
// The name field of MultiClusterService is the same as that of Service name.
// Services with the same name in different clusters are regarded as the same
// service and are associated with the same MultiClusterService.
// MultiClusterService can control the exposure of services to outside multiple
// clusters, and also enable service discovery between clusters.
type MultiClusterService struct {
	metav1.TypeMeta   `json:",inline"`
	metav1.ObjectMeta `json:"metadata,omitempty"`

	// Spec is the desired state of the MultiClusterService.
	// +optional
	Spec MultiClusterServiceSpec `json:"spec,omitempty"`

	// Status is the current state of the MultiClusterService.
	// +optional
	Status corev1.ServiceStatus `json:"status,omitempty"`
}

// MultiClusterServiceSpec is the desired state of the MultiClusterService.
type MultiClusterServiceSpec struct {
	// ExposureMethods specifies how to expose the service referencing by this
	// MultiClusterService.
	//
	// +required
	ExposureMethods []ExposureMethod `json:"exposureMethods"`

	// ExposurePort specifies which port of the referencing service should be
	// exposed.
	// Only required when a service have more than one port.
	// +optional
	ExposurePort ExposurePort `json:"exposurePort,omitempty"`

	// ExposureRange specifies the ranges where the referencing service should
	// be exposed.
	// Only valid and optional in case of ExposureMethods contains CrossCluster.
	// If not set and ExposureMethods contains CrossCluster, all clusters will
	// be selected, that means the referencing serviced will be exposed across
	// all registered clusters.
	// +optional
	ExposureRange ExposureRange `json:"exposureRange,omitempty"`
}

// ExposureMethod describes how to expose the service.
type ExposureMethod string

const (
	// ExposureMethodCrossCluster means a service will be accessible across clusters.
	ExposureMethodCrossCluster ExposureMethod = "CrossCluster"

	// ExposureMethodLoadBalancer means a service will be exposed via an external
	// load balancer.
	ExposureMethodLoadBalancer ExposureMethod = "LoadBalancer"
)

// ExposurePort describes which port will be exposed.
type ExposurePort struct {
	// Name is the name of the port that needs to be exposed within the service.
	// The port name must be the same as that defined in the service.
	// +optional
	Name string `json:"name,omitempty"`

	// Port specifies the exposed service port.
	// +optional
	Port int32 `json:"port,omitempty"`
}

// ExposureRange describes a list of clusters where the service is exposed.
// Now supports selecting cluster by name, leave the room for extend more methods
// such as using label selector.
type ExposureRange struct {
	// ClusterNames is the list of clusters to be selected.
	// +optional
	ClusterNames []string `json:"clusterNames,omitempty"`
}

@GitHubxsy
Copy link
Contributor

#3644 (review)
Exposure prefix is ​​redundant

@XiShanYongYe-Chang XiShanYongYe-Chang force-pushed the add-MultiClusterService-api branch 2 times, most recently from fa9aacf to c5b11dd Compare June 26, 2023 07:38
Copy link
Member

@RainbowMango RainbowMango left a comment

Choose a reason for hiding this comment

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

Echo from https://github.com/karmada-io/karmada/actions/runs/5375174785/jobs/9751322967?pr=3644

13m 38s
Run ct install --debug --helm-extra-args "--timeout 800s"
Installing charts...
>>> helm version --short
>>> git rev-parse --is-inside-work-tree
>>> git merge-base origin/master HEAD
>>> git diff --find-renames --name-only 276428e0eeae90272d473d269[1](https://github.com/karmada-io/karmada/actions/runs/5375174785/jobs/9751322967?pr=3644#step:10:1)c10b1bf1f10d88 -- charts

------------------------------------------------------------------------------------------------------------------------
 Charts to be processed:
------------------------------------------------------------------------------------------------------------------------
 karmada => (version: "0.0.0", path: "charts/karmada")
------------------------------------------------------------------------------------------------------------------------

>>> helm dependency build charts/karmada
Hang tight while we grab the latest from your chart repositories...
...Successfully got an update from the "bitnami" chart repository
Update Complete. ⎈Happy Helming!⎈
Saving 1 charts
Downloading common from repo https://charts.bitnami.com/bitnami
Deleting outdated charts
Installing chart 'karmada => (version: "0.0.0", path: "charts/karmada")'...
Creating namespace 'karmada-xhfmy98ebo'...
>>> kubectl create namespace karmada-xhfmy98ebo
namespace/karmada-xhfmy98ebo created
>>> helm install karmada-xhfmy98ebo charts/karmada --namespace karmada-xhfmy98ebo --wait --timeout 800s
Error: INSTALLATION FAILED: failed pre-install: warning: Hook pre-install karmada/templates/pre-install-job.yaml failed: ConfigMap "karmada-xhfmy98ebo-config" is invalid: []: Too long: must have at most 1048576 bytes
========================================================================================================================
........................................................................................................................
==> Events of namespace karmada-xhfmy98ebo
........................................................................................................................
>>> kubectl get events --output wide --namespace karmada-xhfmy98ebo
No resources found in karmada-xhfmy98ebo namespace.
........................................................................................................................
<== Events of namespace karmada-xhfmy98ebo
........................................................................................................................
>>> kubectl get pods --no-headers --namespace karmada-xhfmy98ebo --selector  --output jsonpath={.items[*].metadata.name}
========================================================================================================================
Deleting release 'karmada-xhfmy98ebo'...
>>> helm uninstall karmada-xhfmy98ebo --namespace karmada-xhfmy98ebo --timeout 800s
Error: uninstallation completed with 1 error(s): timed out waiting for the condition
Error deleting Helm release: Error waiting for process: exit status 1
Deleting namespace 'karmada-xhfmy98ebo'...
>>> kubectl delete namespace karmada-xhfmy98ebo --timeout [18](https://github.com/karmada-io/karmada/actions/runs/5375174785/jobs/9751322967?pr=3644#step:10:19)0s
namespace "karmada-xhfmy98ebo" deleted
>>> kubectl get namespace karmada-xhfmy98ebo
Error: Error installing charts: Error processing charts
Namespace 'karmada-xhfmy98ebo' terminated.
------------------------------------------------------------------------------------------------------------------------
 ✖︎ karmada => (version: "0.0.0", path: "charts/karmada") > Error waiting for process: exit status 1
------------------------------------------------------------------------------------------------------------------------
Error installing charts: Error processing charts
Error: Process completed with exit code 1.

Copy link
Member

@RainbowMango RainbowMango left a comment

Choose a reason for hiding this comment

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

/lgtm

@karmada-bot karmada-bot added the lgtm Indicates that a PR is ready to be merged. label Jun 26, 2023
@XiShanYongYe-Chang
Copy link
Member Author

Hi @calvin0327 can you help take a look of Chart Lint?
/cc @calvin0327

@karmada-bot karmada-bot removed the lgtm Indicates that a PR is ready to be merged. label Jun 26, 2023
@codecov-commenter
Copy link

codecov-commenter commented Jun 26, 2023

Codecov Report

Merging #3644 (9cf3dc5) into master (b005d5b) will decrease coverage by 0.01%.
The diff coverage is n/a.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@            Coverage Diff             @@
##           master    #3644      +/-   ##
==========================================
- Coverage   55.68%   55.68%   -0.01%     
==========================================
  Files         222      222              
  Lines       21195    21195              
==========================================
- Hits        11803    11802       -1     
  Misses       8765     8765              
- Partials      627      628       +1     
Flag Coverage Δ
unittests 55.68% <ø> (-0.01%) ⬇️

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

see 2 files with indirect coverage changes

@XiShanYongYe-Chang XiShanYongYe-Chang force-pushed the add-MultiClusterService-api branch 9 times, most recently from 9cf3dc5 to 779eaa0 Compare June 27, 2023 11:06
@XiShanYongYe-Chang
Copy link
Member Author

CI chart-lint-test error requires #3719 and #3721 to be merged first.

@XiShanYongYe-Chang XiShanYongYe-Chang force-pushed the add-MultiClusterService-api branch 2 times, most recently from 71530fa to 93e62d0 Compare June 27, 2023 14:58
Signed-off-by: changzhen <changzhen5@huawei.com>
Copy link
Member

@RainbowMango RainbowMango left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

@karmada-bot karmada-bot added the lgtm Indicates that a PR is ready to be merged. label Jun 28, 2023
@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 28, 2023
@RainbowMango RainbowMango added this to the v1.7 milestone Jun 28, 2023
@karmada-bot karmada-bot merged commit f267b3d into karmada-io:master Jun 28, 2023
12 checks passed
@XiShanYongYe-Chang XiShanYongYe-Chang deleted the add-MultiClusterService-api branch July 12, 2023 01:45
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/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API lgtm Indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants