proposal:rollout design #489
Conversation
Signed-off-by: LiZhenCheng9527 <lizhencheng6@huawei.com>
✅ Deploy Preview for kurator-dev ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
@LiZhenCheng9527: The label(s) In response to this:
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. |
| // If set it to ture, user need to install the testload himself. | ||
| // If set it to false or leave it blank, Kurator will install the flagger's testload. | ||
| // +optional | ||
| TestLoad bool `json:"testLoad"` |
| // Destination defines the destination for the artifact. | ||
| // If specified, it will override the destination specified at Application level. | ||
| // +optional | ||
| Destination *ApplicationDestination `json:"destination"` |
There was a problem hiding this comment.
BTW, Destination may be refactored in the future. See #439.
|
|
||
| // Kind of the referent | ||
| // +optional | ||
| Kind string `json:"kind,omitempty"` |
There was a problem hiding this comment.
If 'kind' is optional, how can we specify or identify an object
| Port int32 `json:"port"` | ||
|
|
||
| // Port name of the generated Kubernetes service. | ||
| // Defaults to http |
There was a problem hiding this comment.
what does "Defaults to http" means? the defaults PortName is "http"?
There was a problem hiding this comment.
Yes. More information you can refer to canary-service
| // +optional | ||
| Hosts []string `json:"hosts,omitempty"` | ||
|
|
||
| // If enabled, Flagger would generate Istio VirtualServices without hosts and gateway, |
There was a problem hiding this comment.
nit, Annotations should start with the parameter name, followed by its description. For example, 'Delegation [description of Delegation]'
| // +optional | ||
| TrafficPolicy *istiov1alpha3.TrafficPolicy `json:"trafficPolicy,omitempty"` | ||
|
|
||
| // URI match conditions for the generated service. |
There was a problem hiding this comment.
| // URI match conditions for the generated service. | |
| // Match specifies URI match conditions for the generated service. |
| } | ||
| ``` | ||
|
|
||
| Kurator generates horizontalpodautoscaler.autoscaling resources based on `autoscale` configurations as stable and to-be-verified versions. For instance tuning during testing |
There was a problem hiding this comment.
| Kurator generates horizontalpodautoscaler.autoscaling resources based on `autoscale` configurations as stable and to-be-verified versions. For instance tuning during testing | |
| Kurator generates horizontalpodautoscaler.autoscaling resources based on `autoscale` configurations as stable and to-be-verified versions. Make adjustments during testing |
| A good summary is probably at least a paragraph in length. | ||
| --> | ||
|
|
||
| Kurator, as an open-source distributed cloud-native platform, has been pivotal in aiding users to construct their distributed cloud-native infrastructure, thereby facilitating enterprise digital transformation. |
There was a problem hiding this comment.
Do not repeat this, too general.
|
|
||
| By integrating Flagger, we aim to provide users with reliable, fast and unified Continuous Delivery, enabling them to easily distribute code across multiple clusters. | ||
|
|
||
| Base on Flagger, Kurator also offers A/B, Blue/Green and Canary distribution options. Meet the needs of the test verification release. |
There was a problem hiding this comment.
test verification release is this a commonly used phrase?
|
|
||
| Unified configuration distribution only requires the user to declare the desired API state in one place, and Kurator will automatically handle all subsequent operations. | ||
|
|
||
| Kurator deploys different daemons on different nodes in the cluster with different rules for configuration distribution based on the declarations in the Spec. |
There was a problem hiding this comment.
? can not get the intention here
|
|
||
| - **unified Continuous Delivery** | ||
| - Supports unified configuration of releases for multiple clusters. Achieve the deployment configuration of the application to be distributed to the specified single or multiple clusters. | ||
| - Support A/B, Blue/Green, Canary three release validation rules configuration. |
There was a problem hiding this comment.
These three are all deploy strategies, donot mention validation? It is confusing to users
| and make progress. | ||
| --> | ||
|
|
||
| - **Traffic distribution tools that Flagger can use other than istio are not supported** While Flagger is able to support a wide range of traffic distribution tools including istio, nginx for grey scale releases. However, Kuraotr currently only supports unified installation of istio in multiple clusters, and Kurator may implement unified installation of other traffic distribution tools in the future. |
There was a problem hiding this comment.
| - **Traffic distribution tools that Flagger can use other than istio are not supported** While Flagger is able to support a wide range of traffic distribution tools including istio, nginx for grey scale releases. However, Kuraotr currently only supports unified installation of istio in multiple clusters, and Kurator may implement unified installation of other traffic distribution tools in the future. | |
| - **Traffic distribution tools other than istio are not supported** While Flagger is able to support a wide range of traffic distribution tools including istio, nginx for grey scale releases. However, Kurator currently only supports unified management of istio across clusters. Kurator may implement other traffic distribution tools in the future. |
| --> | ||
| The purpose of this proposal is to introduce a unified Continuous Delivery for Kurator that supports A/B, Blue/Green, and Canary.The main objectives of this proposal are as follows: | ||
|
|
||
| Custom Resource Definitions (CRDs): Design CRDs to enable Uniform Continuous Delivery These CRDs will provide a structured approach to defining clusters and different configuration distribution rules to enable uniform configuration distribution. |
|
|
||
| Custom Resource Definitions (CRDs): Design CRDs to enable Uniform Continuous Delivery These CRDs will provide a structured approach to defining clusters and different configuration distribution rules to enable uniform configuration distribution. | ||
|
|
||
| Fleet-Manager Implementation: The Cluster Manager component will be responsible for monitoring the CRDs and performing the defined functions. It will install Flagger on the clusters and handle potential errors or anomalies to ensure smooth operation. |
There was a problem hiding this comment.
Not fleet-manager implementation. You are implementing CD, IIRC, you want to introduce a new component. So it is the implement of XXXX
There was a problem hiding this comment.
I feel Cluster Manager is not a good name. I do not have a good suggestion now, maybe RollOut Engine or Delivery Engine, i am not sure.
| // Delivery defines the Continuous Delivery Configurations to be used. | ||
| // If specified, a uniform Continuous Delivery policy is configured for this installed object. | ||
| // +optional | ||
| Delivery *DeliveryConfig `json:"deliveryPolicy"` |
| Delivery *DeliveryConfig `json:"deliveryPolicy"` | ||
| } | ||
|
|
||
| type DeliveryConfig struct { |
| // +optional | ||
| Autoscale *AutoscaleRef `json:"autoScale,omitempty"` | ||
|
|
||
| // Whether or not the testload is installed by the user. The default is false. |
There was a problem hiding this comment.
| // Whether or not the testload is installed by the user. The default is false. | |
| // Whether or not the testloader is installed by the user. The default is false. |
| Autoscale *AutoscaleRef `json:"autoScale,omitempty"` | ||
|
|
||
| // Whether or not the testload is installed by the user. The default is false. | ||
| // If set it to ture, user need to install the testload himself. |
There was a problem hiding this comment.
| // If set it to ture, user need to install the testload himself. | |
| // If set to true, user needs to install the testloader himself. |
There was a problem hiding this comment.
I think the semantics is negative.
We can make it: testLoader will be installed when this is true
| // +optional | ||
| TestLoad bool `json:"testLoad"` | ||
|
|
||
| // DeliveryPolicy defines the Release Strategy of Object |
There was a problem hiding this comment.
``
| // DeliveryPolicy defines the Release Strategy of Object | |
| // DeliveryPolicy defines the release strategy of XXXX |
There was a problem hiding this comment.
It is not object, it is deployment
| type DeliveryConfig struct { | ||
| // Autoscale defines minreplicas and maxreplicas of horizontal Pod Autoscaler. | ||
| // +optional | ||
| Autoscale *AutoscaleRef `json:"autoScale,omitempty"` |
There was a problem hiding this comment.
It is not clear, how this will influence the rolling out
|
|
||
| // TargetObject specifies what object to deploy the test to. | ||
| // Objects of type deployment or daemonSet. | ||
| TargetObject *TargetObjectReference `json:"targetObject"` |
There was a problem hiding this comment.
Move to the upper struct DeliveryConfig?
This must be specified
There was a problem hiding this comment.
s/TargetObject/WorkloadRef ?
| ```console | ||
| // Note: refer to https://github.com/fluxcd/flagger/blob/main/pkg/apis/flagger/v1beta1/canary.go | ||
| type ServiceConfig struct { | ||
| // Name of the virtual Kubernetes generated by Flagger. |
There was a problem hiding this comment.
virtual Kubernetes means what?
|
|
||
| ```console | ||
| // Note: refer to https://github.com/fluxcd/flagger/blob/main/pkg/apis/flagger/v1beta1/canary.go | ||
| type ServiceConfig struct { |
There was a problem hiding this comment.
is this used for canary svc generation? If so why does not it use the original svc's spec.
|
|
||
| // PortDiscovery adds all container ports to the generated Kubernetes service. | ||
| // Defaults to true | ||
| PortDiscovery bool `json:"portDiscovery"` |
| Primary *CustomMetadata `json:"primary,omitempty"` | ||
|
|
||
| // Canary is the metadata to add to the canary service. | ||
| // +optional | ||
| Canary *CustomMetadata `json:"canary,omitempty"` |
There was a problem hiding this comment.
Not sure i understand. Is this annotation?
LiZhenCheng9527
force-pushed
the
proposal-of-kuratorCD
branch
from
ea1cdda to
c34987f
Compare
Signed-off-by: LiZhenCheng9527 <lizhencheng6@huawei.com>
docs/proposals/rollout/rollout.md
Outdated
| // If set true, Kurator will remove Testloader when delete | ||
| // Application.Spec.ApplicationSyncPolicy.Rollout. | ||
| // Defines to false. | ||
| RemoveTestloaderOnDelete *bool `json:"removeTestloaderOnDelete,omitempty"` |
There was a problem hiding this comment.
assume this can be set simultaneously with TestLoader.
How about adding a struct for testLoader
type TestLoader struct{
Enable bool
RemoveOnDelete bool
}
docs/proposals/rollout/rollout.md
Outdated
| // the presence or absence of content in the rolloutAnalysisCheckTimes field. | ||
| // So can't configure rolloutAnalysisCheckTimes and canaryStrategy at the same time. | ||
| // +optional | ||
| RolloutAnalysisCheckTimes *int `json:"analysisCheckTimes,omitempty"` |
There was a problem hiding this comment.
s/RolloutAnalysisCheckTimes/AnalysisTimes
We have the context in the higher level struct
docs/proposals/rollout/rollout.md
Outdated
| @@ -300,8 +288,39 @@ type TrafficRoutingConfig struct { | |||
| // +optional | |||
| CorsPolicy *istiov1alpha3.CorsPolicy `json:"corsPolicy,omitempty"` | |||
|
|
|||
| // RolloutAnalysisCheckTimes defines the number of traffic analysis checks to run for A/B Testing and Blue/Green | |||
| // If set "rolloutAnalysisCheckTimes: 10". It means Kurator will checks the preview service ten times. | |||
| // Note: Kurator determines whether Blue/Green and A/B or canary related processing is required based on | |||
There was a problem hiding this comment.
Prefer making use of CanaryStrategy to indicate if it is canary release
docs/proposals/rollout/rollout.md
Outdated
| // the presence or absence of content in the rolloutAnalysisCheckTimes field. | ||
| // So can't configure rolloutAnalysisCheckTimes and canaryStrategy at the same time. | ||
| // +optional | ||
| RolloutAnalysisCheckTimes *int `json:"analysisCheckTimes,omitempty"` |
There was a problem hiding this comment.
Make sure you want to make default value not equal to 0 before using pointer int
docs/proposals/rollout/rollout.md
Outdated
| // +optional | ||
| RolloutAnalysisCheckTimes *int `json:"analysisCheckTimes,omitempty"` | ||
|
|
||
| // Match conditions of A/B HTTP header. |
docs/proposals/rollout/rollout.md
Outdated
| @@ -300,8 +288,39 @@ type TrafficRoutingConfig struct { | |||
| // +optional | |||
| CorsPolicy *istiov1alpha3.CorsPolicy `json:"corsPolicy,omitempty"` | |||
|
|
|||
| // RolloutAnalysisCheckTimes defines the number of traffic analysis checks to run for A/B Testing and Blue/Green | |||
There was a problem hiding this comment.
Please check all Blue/Green and A/B, usually it is with A/B testing ·Blue/Green Deployment
docs/proposals/rollout/rollout.md
Outdated
| @@ -386,12 +398,57 @@ type Metric struct { | |||
| ThresholdRange *CanaryThresholdRange `json:"thresholdRange,omitempty"` | |||
|
|
|||
| // TemplateRef references a metric template object | |||
| // The metric can be custom by users. | |||
docs/proposals/rollout/rollout.md
Outdated
| Name string `json:"name,omitempty"` | ||
|
|
||
| // Namespace of MetricTemplate | ||
| // More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/namespaces | ||
| Namespace string `json:"namespace,omitempty"` | ||
|
|
||
| // MetricProvider defines the metric provider of metricTemplate. | ||
| // Kurator now only supports `Prometheus` as metricProvider. | ||
| // Default is prometheus | ||
| MetricProvider string `json:"metricProvider,omitempty"` |
There was a problem hiding this comment.
It is not clear how these fields work, i would suggest removing MetricTemplate ATM
docs/proposals/rollout/rollout.md
Outdated
| // The traffic analysis can be extended with webhooks. | ||
| // Kurator will call each webhook URL and determine from the response status code (HTTP 2xx) | ||
| // if the test is failing or not. | ||
| // Kurator calls the testloader via webhook to generate a load of traffic accessing service. |
There was a problem hiding this comment.
a load of traffic accessing service >
Signed-off-by: LiZhenCheng9527 <lizhencheng6@huawei.com>
LiZhenCheng9527
force-pushed
the
proposal-of-kuratorCD
branch
from
2537a6d to
9b47d90
Compare
docs/proposals/rollout/rollout.md
Outdated
| @@ -215,7 +224,7 @@ type RolloutPolicy struct { | |||
| // preview deployment to make progress before it is considered to be failed. | |||
| // Defaults to 600. | |||
| // +optional | |||
| RolloutTimeoutSeconds *int `json:"rolloutTimeoutSeconds,omitempty"` | |||
| RolloutTimeoutSeconds int `json:"rolloutTimeoutSeconds,omitempty"` | |||
There was a problem hiding this comment.
If you want to make it default to 600s, this should be *int
docs/proposals/rollout/rollout.md
Outdated
| // Note: Kurator determines whether A/B Testing and Blue/Green Deployment or Canary Deployment | ||
| // related processing is required based on the presence or absence of content in the analysisTimes field. |
There was a problem hiding this comment.
please reword this sentence. can you try totranslate into Chinese using dictionary
docs/proposals/rollout/rollout.md
Outdated
| @@ -387,68 +397,19 @@ type TrafficAnalysis struct { | |||
|
|
|||
| type Metric struct { | |||
| // Name of the metric. | |||
| // User also can use name point to custom metric checks. | |||
| // The metrics now used in kurator are `request-success-rate` and `request-duration` | |||
There was a problem hiding this comment.
Currently supported metric are xxx and xxx
docs/proposals/rollout/rollout.md
Outdated
| @@ -387,68 +397,19 @@ type TrafficAnalysis struct { | |||
|
|
|||
| type Metric struct { | |||
| // Name of the metric. | |||
| // User also can use name point to custom metric checks. | |||
| // The metrics now used in kurator are `request-success-rate` and `request-duration` | |||
| // `request-success-rate` calculates the ratio of successful request to total request for a | |||
There was a problem hiding this comment.
request-success-rate indicates the successful request ratio during this checking interval
docs/proposals/rollout/rollout.md
Outdated
| // The metrics now used in kurator are `request-success-rate` and `request-duration` | ||
| // `request-success-rate` calculates the ratio of successful request to total request for a | ||
| // checkIntervalSeconds time. It returns a value from 0 to 100. | ||
| // `request-duration` returns the elapsed time for the longest request within a CheckIntervalSeconds. |
There was a problem hiding this comment.
the longest request, i am sure this is not right
CheckIntervalSeconds is not consitent with below IntervalSeconds
Signed-off-by: LiZhenCheng9527 <lizhencheng6@huawei.com>
LiZhenCheng9527
force-pushed
the
proposal-of-kuratorCD
branch
from
89bbdfa to
4b27191
Compare
docs/proposals/rollout/rollout.md
Outdated
| // Currently supported metric are `request-success-rate` and `request-duration`. | ||
| // `request-success-rate` indicates the successful request ratio during this checking intervalSeconds. | ||
| // It returns a value from 0 to 100. | ||
| // `request-duration` returns the P99 latency within intervalSeconds. |
There was a problem hiding this comment.
request-duration indicates P99 latency of the requests during the check interval
docs/proposals/rollout/rollout.md
Outdated
| // Defaults to 60. | ||
| IntervalSeconds *int `json:"intervalSeconds,omitempty"` | ||
|
|
||
| // Range value accepted for this metric |
There was a problem hiding this comment.
| // Range value accepted for this metric | |
| // Valid range accepted for this metric |
Signed-off-by: LiZhenCheng9527 <lizhencheng6@huawei.com>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: hzxuzhonghu 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 |
What type of PR is this?
/kind documentation
What this PR does / why we need it:
a part of #435