From 3eeb7b4ddc2c2e605aa826dd5d65af421a6c4032 Mon Sep 17 00:00:00 2001 From: myname4423 <57184070+myname4423@users.noreply.github.com> Date: Sat, 25 May 2024 09:45:24 +0800 Subject: [PATCH 1/3] modify the helm version from latest to v3.14.0 (#215) Signed-off-by: yunbo Co-authored-by: yunbo --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 87dac730..b5ffdd6f 100644 --- a/Makefile +++ b/Makefile @@ -108,7 +108,7 @@ ginkgo: ## Download ginkgo locally if necessary. HELM = $(shell pwd)/bin/helm helm: ## Download helm locally if necessary. - $(call go-get-tool,$(HELM),helm.sh/helm/v3/cmd/helm@latest) + $(call go-get-tool,$(HELM),helm.sh/helm/v3/cmd/helm@v3.14.0) # go-get-tool will 'go get' any package $2 and install it to $1. PROJECT_DIR := $(shell dirname $(abspath $(lastword $(MAKEFILE_LIST)))) From 62794dc883d0b26c27f7423c019202c0988fb03d Mon Sep 17 00:00:00 2001 From: Jiajing LU Date: Wed, 12 Jun 2024 13:21:42 +0800 Subject: [PATCH 2/3] Support Path and QueryParams in http route matches (#204) * support queryparams for gateway api Signed-off-by: Megrez Lu * support mse Signed-off-by: Megrez Lu * support path and queryParams Signed-off-by: Megrez Lu * fix lint Signed-off-by: Megrez Lu * fix testcase Signed-off-by: Megrez Lu * fix manifests Signed-off-by: Megrez Lu * do not provide default value for path Signed-off-by: Megrez Lu * Allow Not generating Canary Service && Fixed a bug caused by NOT considering case-insensitivity. (#200) * Fixed a bug caused by NOT considering case-insensitivity. Signed-off-by: yunbo * add DisableGenerateCanaryService for CanaryStrategy amend1: update crd yaml amend2: add DisableGenerateCanaryService for v1alpha1 Signed-off-by: yunbo --------- Signed-off-by: yunbo Co-authored-by: yunbo Signed-off-by: Megrez Lu * revert test images Signed-off-by: Megrez Lu * polish comments Signed-off-by: Megrez Lu * add gateway api tests Signed-off-by: Megrez Lu * fix MSE cases Signed-off-by: Megrez Lu * update golang lint ci Signed-off-by: Megrez Lu * regenerate manifests Signed-off-by: Megrez Lu * remove generic usage Signed-off-by: Megrez Lu * update istio lua script Signed-off-by: Megrez Lu * update v1alpha1 in e2e to v1beta1 Signed-off-by: Megrez Lu * fix cases Signed-off-by: Megrez Lu * refactor istio case to include queryParams and path Signed-off-by: Megrez Lu * fix cloneset issue Signed-off-by: Megrez Lu * fix typo Signed-off-by: Megrez Lu * revert images Signed-off-by: Megrez Lu --------- Signed-off-by: Megrez Lu Signed-off-by: yunbo Co-authored-by: myname4423 <57184070+myname4423@users.noreply.github.com> Co-authored-by: yunbo --- .github/workflows/ci.yaml | 4 +- .github/workflows/e2e-custom.yaml | 2 +- api/v1beta1/rollout_types.go | 36 +- api/v1beta1/zz_generated.deepcopy.go | 12 + .../bases/rollouts.kruise.io_rollouts.yaml | 94 +++- .../VirtualService/trafficRouting.lua | 34 +- .../trafficrouting_ingress/mse.lua | 36 +- .../custom_network_provider_test.go | 102 ++++- .../VirtualService/trafficRouting.lua | 33 +- pkg/trafficrouting/network/gateway/gateway.go | 35 +- .../network/gateway/gateway_test.go | 431 +++++++++++++++++- .../network/ingress/ingress_test.go | 156 ++++++- pkg/util/slices_utils.go | 28 ++ test/e2e/rollout_test.go | 44 +- .../rollout_with_trafficrouting.yaml | 23 +- 15 files changed, 991 insertions(+), 79 deletions(-) create mode 100644 pkg/util/slices_utils.go diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 3d6c1a6b..7f720436 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -11,7 +11,7 @@ on: env: # Common versions GO_VERSION: '1.19' - GOLANGCI_VERSION: 'v1.42' + GOLANGCI_VERSION: 'v1.52' jobs: @@ -36,7 +36,7 @@ jobs: run: | make generate - name: Lint golang code - uses: golangci/golangci-lint-action@v2 + uses: golangci/golangci-lint-action@v6 with: version: ${{ env.GOLANGCI_VERSION }} args: --verbose diff --git a/.github/workflows/e2e-custom.yaml b/.github/workflows/e2e-custom.yaml index 4803b098..cb730459 100644 --- a/.github/workflows/e2e-custom.yaml +++ b/.github/workflows/e2e-custom.yaml @@ -70,7 +70,7 @@ jobs: kubectl apply -f ./test/e2e/test_data/customNetworkProvider/lua_script_configmap.yaml make ginkgo set +e - ./bin/ginkgo -timeout 60m -v --focus='Canary rollout with custon network provider' test/e2e + ./bin/ginkgo -timeout 60m -v --focus='Canary rollout with custom network provider' test/e2e retVal=$? # kubectl get pod -n kruise-rollout --no-headers | grep manager | awk '{print $1}' | xargs kubectl logs -n kruise-rollout restartCount=$(kubectl get pod -n kruise-rollout --no-headers | awk '{print $4}') diff --git a/api/v1beta1/rollout_types.go b/api/v1beta1/rollout_types.go index 25c91106..fcf61682 100644 --- a/api/v1beta1/rollout_types.go +++ b/api/v1beta1/rollout_types.go @@ -146,19 +146,47 @@ type TrafficRoutingStrategy struct { // // +optional RequestHeaderModifier *gatewayv1beta1.HTTPRequestHeaderFilter `json:"requestHeaderModifier,omitempty"` - // Matches define conditions used for matching the incoming HTTP requests to canary service. - // Each match is independent, i.e. this rule will be matched if **any** one of the matches is satisfied. - // If Gateway API, current only support one match. - // And cannot support both weight and matches, if both are configured, then matches takes precedence. + // Matches define conditions used for matching incoming HTTP requests to the canary service. + // Each match is independent, i.e. this rule will be matched as long as **any** one of the matches is satisfied. + // + // It cannot support Traffic (weight-based routing) and Matches simultaneously, if both are configured. + // In such cases, Matches takes precedence. Matches []HttpRouteMatch `json:"matches,omitempty"` } type HttpRouteMatch struct { + // Path specifies a HTTP request path matcher. + // Supported list: + // - Istio: https://istio.io/latest/docs/reference/config/networking/virtual-service/#HTTPMatchRequest + // - GatewayAPI: If path is defined, the whole HttpRouteMatch will be used as a standalone matcher + // + // +optional + Path *gatewayv1beta1.HTTPPathMatch `json:"path,omitempty"` + // Headers specifies HTTP request header matchers. Multiple match values are // ANDed together, meaning, a request must match all the specified headers // to select the route. + // + // +listType=map + // +listMapKey=name + // +optional // +kubebuilder:validation:MaxItems=16 Headers []gatewayv1beta1.HTTPHeaderMatch `json:"headers,omitempty"` + + // QueryParams specifies HTTP query parameter matchers. Multiple match + // values are ANDed together, meaning, a request must match all the + // specified query parameters to select the route. + // Supported list: + // - Istio: https://istio.io/latest/docs/reference/config/networking/virtual-service/#HTTPMatchRequest + // - MSE Ingress: https://help.aliyun.com/zh/ack/ack-managed-and-ack-dedicated/user-guide/annotations-supported-by-mse-ingress-gateways-1 + // Header/Cookie > QueryParams + // - Gateway API + // + // +listType=map + // +listMapKey=name + // +optional + // +kubebuilder:validation:MaxItems=16 + QueryParams []gatewayv1beta1.HTTPQueryParamMatch `json:"queryParams,omitempty"` } // RolloutPause defines a pause stage for a rollout diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go index 408b544f..2201cc70 100644 --- a/api/v1beta1/zz_generated.deepcopy.go +++ b/api/v1beta1/zz_generated.deepcopy.go @@ -295,6 +295,11 @@ func (in *GatewayTrafficRouting) DeepCopy() *GatewayTrafficRouting { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *HttpRouteMatch) DeepCopyInto(out *HttpRouteMatch) { *out = *in + if in.Path != nil { + in, out := &in.Path, &out.Path + *out = new(apisv1beta1.HTTPPathMatch) + (*in).DeepCopyInto(*out) + } if in.Headers != nil { in, out := &in.Headers, &out.Headers *out = make([]apisv1beta1.HTTPHeaderMatch, len(*in)) @@ -302,6 +307,13 @@ func (in *HttpRouteMatch) DeepCopyInto(out *HttpRouteMatch) { (*in)[i].DeepCopyInto(&(*out)[i]) } } + if in.QueryParams != nil { + in, out := &in.QueryParams, &out.QueryParams + *out = make([]apisv1beta1.HTTPQueryParamMatch, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new HttpRouteMatch. diff --git a/config/crd/bases/rollouts.kruise.io_rollouts.yaml b/config/crd/bases/rollouts.kruise.io_rollouts.yaml index 62c072a6..ad1f51a8 100644 --- a/config/crd/bases/rollouts.kruise.io_rollouts.yaml +++ b/config/crd/bases/rollouts.kruise.io_rollouts.yaml @@ -622,13 +622,13 @@ spec: description: CanaryStep defines a step of a canary workload. properties: matches: - description: Matches define conditions used for matching - the incoming HTTP requests to canary service. Each + description: "Matches define conditions used for matching + incoming HTTP requests to the canary service. Each match is independent, i.e. this rule will be matched - if **any** one of the matches is satisfied. If Gateway - API, current only support one match. And cannot support - both weight and matches, if both are configured, then - matches takes precedence. + as long as **any** one of the matches is satisfied. + \n It cannot support Traffic (weight-based routing) + and Matches simultaneously, if both are configured. + In such cases, Matches takes precedence." items: properties: headers: @@ -690,6 +690,88 @@ spec: type: object maxItems: 16 type: array + x-kubernetes-list-map-keys: + - name + x-kubernetes-list-type: map + path: + description: 'Path specifies a HTTP request path + matcher. Supported list: - Istio: https://istio.io/latest/docs/reference/config/networking/virtual-service/#HTTPMatchRequest + - GatewayAPI: If path is defined, the whole + HttpRouteMatch will be used as a standalone + matcher' + properties: + type: + default: PathPrefix + description: "Type specifies how to match + against the path Value. \n Support: Core + (Exact, PathPrefix) \n Support: Custom (RegularExpression)" + enum: + - Exact + - PathPrefix + - RegularExpression + type: string + value: + default: / + description: Value of the HTTP path to match + against. + maxLength: 1024 + type: string + type: object + queryParams: + description: 'QueryParams specifies HTTP query + parameter matchers. Multiple match values are + ANDed together, meaning, a request must match + all the specified query parameters to select + the route. Supported list: - Istio: https://istio.io/latest/docs/reference/config/networking/virtual-service/#HTTPMatchRequest + - MSE Ingress: https://help.aliyun.com/zh/ack/ack-managed-and-ack-dedicated/user-guide/annotations-supported-by-mse-ingress-gateways-1 Header/Cookie + > QueryParams - Gateway API' + items: + description: HTTPQueryParamMatch describes how + to select a HTTP route by matching HTTP query + parameters. + properties: + name: + description: "Name is the name of the HTTP + query param to be matched. This must be + an exact string match. (See https://tools.ietf.org/html/rfc7230#section-2.7.3). + \n If multiple entries specify equivalent + query param names, only the first entry + with an equivalent name MUST be considered + for a match. Subsequent entries with an + equivalent query param name MUST be ignored." + maxLength: 256 + minLength: 1 + type: string + type: + default: Exact + description: "Type specifies how to match + against the value of the query parameter. + \n Support: Extended (Exact) \n Support: + Custom (RegularExpression) \n Since RegularExpression + QueryParamMatchType has custom conformance, + implementations can support POSIX, PCRE + or any other dialects of regular expressions. + Please read the implementation's documentation + to determine the supported dialect." + enum: + - Exact + - RegularExpression + type: string + value: + description: Value is the value of HTTP + query param to be matched. + maxLength: 1024 + minLength: 1 + type: string + required: + - name + - value + type: object + maxItems: 16 + type: array + x-kubernetes-list-map-keys: + - name + x-kubernetes-list-type: map type: object type: array pause: diff --git a/lua_configuration/networking.istio.io/VirtualService/trafficRouting.lua b/lua_configuration/networking.istio.io/VirtualService/trafficRouting.lua index 1581c794..956d67f8 100644 --- a/lua_configuration/networking.istio.io/VirtualService/trafficRouting.lua +++ b/lua_configuration/networking.istio.io/VirtualService/trafficRouting.lua @@ -47,26 +47,40 @@ function GenerateRoutesWithMatches(spec, matches, stableService, canaryService) for _, match in ipairs(matches) do local route = {} route["match"] = {} + + local vsMatch = {} for key, value in pairs(match) do - local vsMatch = {} - vsMatch[key] = {} - for _, rule in ipairs(value) do + if key == "path" then + vsMatch["uri"] = {} + local rule = value if rule["type"] == "RegularExpression" then matchType = "regex" elseif rule["type"] == "Exact" then matchType = "exact" - elseif rule["type"] == "Prefix" then + elseif rule["type"] == "PathPrefix" then matchType = "prefix" end - if key == "headers" then - vsMatch[key][rule["name"]] = {} - vsMatch[key][rule["name"]][matchType] = rule.value - else - vsMatch[key][matchType] = rule.value + vsMatch["uri"][matchType] = rule.value + else + vsMatch[key] = {} + for _, rule in ipairs(value) do + if rule["type"] == "RegularExpression" then + matchType = "regex" + elseif rule["type"] == "Exact" then + matchType = "exact" + elseif rule["type"] == "Prefix" then + matchType = "prefix" + end + if key == "headers" or key == "queryParams" then + vsMatch[key][rule["name"]] = {} + vsMatch[key][rule["name"]][matchType] = rule.value + else + vsMatch[key][matchType] = rule.value + end end end - table.insert(route["match"], vsMatch) end + table.insert(route["match"], vsMatch) route.route = { { destination = {} diff --git a/lua_configuration/trafficrouting_ingress/mse.lua b/lua_configuration/trafficrouting_ingress/mse.lua index 46b0a010..d39ba7dc 100644 --- a/lua_configuration/trafficrouting_ingress/mse.lua +++ b/lua_configuration/trafficrouting_ingress/mse.lua @@ -10,6 +10,10 @@ annotations["nginx.ingress.kubernetes.io/canary-by-cookie"] = nil annotations["nginx.ingress.kubernetes.io/canary-by-header"] = nil annotations["nginx.ingress.kubernetes.io/canary-by-header-pattern"] = nil annotations["nginx.ingress.kubernetes.io/canary-by-header-value"] = nil +-- MSE extended annotations +annotations["mse.ingress.kubernetes.io/canary-by-query"] = nil +annotations["mse.ingress.kubernetes.io/canary-by-query-pattern"] = nil +annotations["mse.ingress.kubernetes.io/canary-by-query-value"] = nil annotations["nginx.ingress.kubernetes.io/canary-weight"] = nil if ( obj.weight ~= "-1" ) then @@ -33,18 +37,30 @@ then return annotations end for _,match in ipairs(obj.matches) do - header = match.headers[1] - if ( header.name == "canary-by-cookie" ) - then - annotations["nginx.ingress.kubernetes.io/canary-by-cookie"] = header.value - else - annotations["nginx.ingress.kubernetes.io/canary-by-header"] = header.name - if ( header.type == "RegularExpression" ) + if match.headers and next(match.headers) ~= nil then + header = match.headers[1] + if ( header.name == "canary-by-cookie" ) then - annotations["nginx.ingress.kubernetes.io/canary-by-header-pattern"] = header.value + annotations["nginx.ingress.kubernetes.io/canary-by-cookie"] = header.value else - annotations["nginx.ingress.kubernetes.io/canary-by-header-value"] = header.value + annotations["nginx.ingress.kubernetes.io/canary-by-header"] = header.name + if ( header.type == "RegularExpression" ) + then + annotations["nginx.ingress.kubernetes.io/canary-by-header-pattern"] = header.value + else + annotations["nginx.ingress.kubernetes.io/canary-by-header-value"] = header.value + end + end + end + if match.queryParams and next(match.queryParams) ~= nil then + queryParam = match.queryParams[1] + annotations["nginx.ingress.kubernetes.io/canary-by-query"] = queryParam.name + if ( queryParam.type == "RegularExpression" ) + then + annotations["nginx.ingress.kubernetes.io/canary-by-query-pattern"] = queryParam.value + else + annotations["nginx.ingress.kubernetes.io/canary-by-query-value"] = queryParam.value end end end -return annotations +return annotations \ No newline at end of file diff --git a/pkg/trafficrouting/network/customNetworkProvider/custom_network_provider_test.go b/pkg/trafficrouting/network/customNetworkProvider/custom_network_provider_test.go index f263c177..f27d9400 100644 --- a/pkg/trafficrouting/network/customNetworkProvider/custom_network_provider_test.go +++ b/pkg/trafficrouting/network/customNetworkProvider/custom_network_provider_test.go @@ -32,6 +32,7 @@ import ( "github.com/openkruise/rollouts/pkg/util" "github.com/openkruise/rollouts/pkg/util/configuration" "github.com/openkruise/rollouts/pkg/util/luamanager" + "github.com/stretchr/testify/assert" lua "github.com/yuin/gopher-lua" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -45,6 +46,7 @@ import ( luajson "layeh.com/gopher-json" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" + gatewayv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1" "sigs.k8s.io/yaml" ) @@ -229,7 +231,7 @@ func checkEqual(cli client.Client, t *testing.T, expect *unstructured.Unstructur fmt.Println(util.DumpJSON(obj.GetAnnotations()), util.DumpJSON(expect.GetAnnotations())) t.Fatalf("expect(%s), but get(%s)", util.DumpJSON(expect.GetAnnotations()), util.DumpJSON(obj.GetAnnotations())) } - if util.DumpJSON(expect.Object["spec"]) != util.DumpJSON(obj.Object["spec"]) { + if !assert.JSONEq(t, util.DumpJSON(expect.Object["spec"]), util.DumpJSON(obj.Object["spec"])) { t.Fatalf("expect(%s), but get(%s)", util.DumpJSON(expect.Object["spec"]), util.DumpJSON(obj.Object["spec"])) } } @@ -245,7 +247,7 @@ func TestEnsureRoutes(t *testing.T) { expectUnstructureds func() []*unstructured.Unstructured }{ { - name: "test1, do traffic routing for VirtualService and DestinationRule", + name: "Do weight-based traffic routing for VirtualService and DestinationRule", getRoutes: func() *v1beta1.TrafficRoutingStrategy { return &v1beta1.TrafficRoutingStrategy{ Traffic: utilpointer.String("5%"), @@ -316,6 +318,102 @@ func TestEnsureRoutes(t *testing.T) { return done, hasError }, }, + { + name: "Do header/queryParam-based traffic routing for VirtualService and DestinationRule", + getRoutes: func() *v1beta1.TrafficRoutingStrategy { + pathTypePrefix := gatewayv1beta1.PathMatchPathPrefix + headerTypeExact := gatewayv1beta1.HeaderMatchExact + queryParamRegex := gatewayv1beta1.QueryParamMatchRegularExpression + return &v1beta1.TrafficRoutingStrategy{ + Matches: []v1beta1.HttpRouteMatch{ + { + Path: &gatewayv1beta1.HTTPPathMatch{ + Type: &pathTypePrefix, + Value: utilpointer.String("/api/v2"), + }, + Headers: []gatewayv1beta1.HTTPHeaderMatch{ + { + Type: &headerTypeExact, + Name: "user_id", + Value: "123456", + }, + }, + QueryParams: []gatewayv1beta1.HTTPQueryParamMatch{ + { + Type: &queryParamRegex, + Name: "user_id", + Value: "123*", + }, + }, + }, + }, + } + }, + getUnstructureds: func() []*unstructured.Unstructured { + objects := make([]*unstructured.Unstructured, 0) + u := &unstructured.Unstructured{} + _ = u.UnmarshalJSON([]byte(virtualServiceDemo)) + u.SetAPIVersion("networking.istio.io/v1alpha3") + objects = append(objects, u) + + u = &unstructured.Unstructured{} + _ = u.UnmarshalJSON([]byte(destinationRuleDemo)) + u.SetAPIVersion("networking.istio.io/v1alpha3") + objects = append(objects, u) + return objects + }, + getConfig: func() Config { + return Config{ + Key: "rollout-demo", + StableService: "echoserver", + CanaryService: "echoserver-canary", + TrafficConf: []v1beta1.ObjectRef{ + { + APIVersion: "networking.istio.io/v1alpha3", + Kind: "VirtualService", + Name: "echoserver", + }, + { + APIVersion: "networking.istio.io/v1alpha3", + Kind: "DestinationRule", + Name: "dr-demo", + }, + }, + } + }, + expectUnstructureds: func() []*unstructured.Unstructured { + objects := make([]*unstructured.Unstructured, 0) + u := &unstructured.Unstructured{} + _ = u.UnmarshalJSON([]byte(virtualServiceDemo)) + annotations := map[string]string{ + OriginalSpecAnnotation: `{"spec":{"hosts":["echoserver.example.com"],"http":[{"route":[{"destination":{"host":"echoserver"}}]}]},"annotations":{"virtual":"test"}}`, + "virtual": "test", + } + u.SetAnnotations(annotations) + specStr := `{"hosts":["echoserver.example.com"],"http":[{"match":[{"headers":{"user_id":{"exact":"123456"}},"queryParams":{"user_id":{"regex":"123*"}},"uri":{"prefix":"/api/v2"}}],"route":[{"destination":{"host":"echoserver-canary"}}]},{"route":[{"destination":{"host":"echoserver"}}]}]}` + var spec interface{} + _ = json.Unmarshal([]byte(specStr), &spec) + u.Object["spec"] = spec + objects = append(objects, u) + + u = &unstructured.Unstructured{} + _ = u.UnmarshalJSON([]byte(destinationRuleDemo)) + annotations = map[string]string{ + OriginalSpecAnnotation: `{"spec":{"host":"mockb","subsets":[{"labels":{"version":"base"},"name":"version-base"}],"trafficPolicy":{"loadBalancer":{"simple":"ROUND_ROBIN"}}}}`, + } + u.SetAnnotations(annotations) + specStr = `{"host":"mockb","subsets":[{"labels":{"version":"base"},"name":"version-base"},{"labels":{"istio.service.tag":"gray"},"name":"canary"}],"trafficPolicy":{"loadBalancer":{"simple":"ROUND_ROBIN"}}}` + _ = json.Unmarshal([]byte(specStr), &spec) + u.Object["spec"] = spec + objects = append(objects, u) + return objects + }, + expectState: func() (bool, bool) { + done := false + hasError := false + return done, hasError + }, + }, { name: "test2, do traffic routing but failed to execute lua", getRoutes: func() *v1beta1.TrafficRoutingStrategy { diff --git a/pkg/trafficrouting/network/customNetworkProvider/lua_configuration/networking.istio.io/VirtualService/trafficRouting.lua b/pkg/trafficrouting/network/customNetworkProvider/lua_configuration/networking.istio.io/VirtualService/trafficRouting.lua index d4d52c23..6db36933 100644 --- a/pkg/trafficrouting/network/customNetworkProvider/lua_configuration/networking.istio.io/VirtualService/trafficRouting.lua +++ b/pkg/trafficrouting/network/customNetworkProvider/lua_configuration/networking.istio.io/VirtualService/trafficRouting.lua @@ -103,26 +103,39 @@ function GenerateMatchedRoutes(spec, matches, stableService, canaryService, stab local route = {} route["match"] = {} + local vsMatch = {} for key, value in pairs(match) do - local vsMatch = {} - vsMatch[key] = {} - for _, rule in ipairs(value) do + if key == "path" then + vsMatch["uri"] = {} + local rule = value if rule["type"] == "RegularExpression" then matchType = "regex" elseif rule["type"] == "Exact" then matchType = "exact" - elseif rule["type"] == "Prefix" then + elseif rule["type"] == "PathPrefix" then matchType = "prefix" end - if key == "headers" then - vsMatch[key][rule["name"]] = {} - vsMatch[key][rule["name"]][matchType] = rule.value - else - vsMatch[key][matchType] = rule.value + vsMatch["uri"][matchType] = rule.value + else + vsMatch[key] = {} + for _, rule in ipairs(value) do + if rule["type"] == "RegularExpression" then + matchType = "regex" + elseif rule["type"] == "Exact" then + matchType = "exact" + elseif rule["type"] == "Prefix" then + matchType = "prefix" + end + if key == "headers" or key == "queryParams" then + vsMatch[key][rule["name"]] = {} + vsMatch[key][rule["name"]][matchType] = rule.value + else + vsMatch[key][matchType] = rule.value + end end end - table.insert(route["match"], vsMatch) end + table.insert(route["match"], vsMatch) route.route = { { destination = {} diff --git a/pkg/trafficrouting/network/gateway/gateway.go b/pkg/trafficrouting/network/gateway/gateway.go index b3ddba4c..208c76fa 100644 --- a/pkg/trafficrouting/network/gateway/gateway.go +++ b/pkg/trafficrouting/network/gateway/gateway.go @@ -153,9 +153,15 @@ func (r *gatewayController) buildDesiredHTTPRoute(rules []gatewayv1beta1.HTTPRou return r.buildCanaryWeightHttpRoutes(rules, weight) } -func (r *gatewayController) buildCanaryHeaderHttpRoutes(rules []gatewayv1beta1.HTTPRouteRule, matchs []v1beta1.HttpRouteMatch) []gatewayv1beta1.HTTPRouteRule { +func (r *gatewayController) buildCanaryHeaderHttpRoutes(rules []gatewayv1beta1.HTTPRouteRule, matches []v1beta1.HttpRouteMatch) []gatewayv1beta1.HTTPRouteRule { var desired []gatewayv1beta1.HTTPRouteRule - var canarys []gatewayv1beta1.HTTPRouteRule + var canaries []gatewayv1beta1.HTTPRouteRule + pathMatches := util.FilterHttpRouteMatch(matches, func(match v1beta1.HttpRouteMatch) bool { + return match.Path != nil + }) + nonPathMatches := util.FilterHttpRouteMatch(matches, func(match v1beta1.HttpRouteMatch) bool { + return match.Path == nil + }) for i := range rules { rule := rules[i] if _, canaryRef := getServiceBackendRef(rule, r.conf.CanaryService); canaryRef != nil { @@ -172,18 +178,35 @@ func (r *gatewayController) buildCanaryHeaderHttpRoutes(rules []gatewayv1beta1.H canaryRule.BackendRefs = []gatewayv1beta1.HTTPBackendRef{*canaryRef} // set canary headers in httpRoute var newMatches []gatewayv1beta1.HTTPRouteMatch + for _, pathMatch := range pathMatches { + newMatches = append(newMatches, gatewayv1beta1.HTTPRouteMatch{ + Path: pathMatch.Path, + Headers: pathMatch.Headers, + QueryParams: pathMatch.QueryParams, + }) + } + // reset pathMatches + pathMatches = nil + if len(nonPathMatches) == 0 && len(newMatches) == 0 { + continue + } for j := range canaryRule.Matches { canaryRuleMatch := &canaryRule.Matches[j] - for k := range matchs { + for k := range nonPathMatches { canaryRuleMatchBase := *canaryRuleMatch - canaryRuleMatchBase.Headers = append(canaryRuleMatchBase.Headers, matchs[k].Headers...) + if len(matches[k].Headers) > 0 { + canaryRuleMatchBase.Headers = append(canaryRuleMatchBase.Headers, matches[k].Headers...) + } + if len(matches[k].QueryParams) > 0 { + canaryRuleMatchBase.QueryParams = append(canaryRuleMatchBase.QueryParams, matches[k].QueryParams...) + } newMatches = append(newMatches, canaryRuleMatchBase) } } canaryRule.Matches = newMatches - canarys = append(canarys, *canaryRule) + canaries = append(canaries, *canaryRule) } - desired = append(desired, canarys...) + desired = append(desired, canaries...) return desired } diff --git a/pkg/trafficrouting/network/gateway/gateway_test.go b/pkg/trafficrouting/network/gateway/gateway_test.go index 4d0a8a8d..648b6c0e 100644 --- a/pkg/trafficrouting/network/gateway/gateway_test.go +++ b/pkg/trafficrouting/network/gateway/gateway_test.go @@ -134,7 +134,7 @@ func TestBuildDesiredHTTPRoute(t *testing.T) { desiredRules func() []gatewayv1beta1.HTTPRouteRule }{ { - name: "test1 headers", + name: "test headers", getRouteRules: func() []gatewayv1beta1.HTTPRouteRule { rules := routeDemo.DeepCopy().Spec.Rules return rules @@ -310,6 +310,435 @@ func TestBuildDesiredHTTPRoute(t *testing.T) { return rules }, }, + { + name: "test query params", + getRouteRules: func() []gatewayv1beta1.HTTPRouteRule { + rules := routeDemo.DeepCopy().Spec.Rules + return rules + }, + getRoutes: func() (*int32, []v1beta1.HttpRouteMatch) { + iType := gatewayv1beta1.QueryParamMatchRegularExpression + return nil, []v1beta1.HttpRouteMatch{ + // queryparams + { + QueryParams: []gatewayv1beta1.HTTPQueryParamMatch{ + { + Name: "user_id", + Value: "123*", + Type: &iType, + }, + { + Name: "canary", + Value: "true", + }, + }, + }, { + QueryParams: []gatewayv1beta1.HTTPQueryParamMatch{ + { + Name: "user_id", + Value: "234*", + Type: &iType, + }, + { + Name: "canary", + Value: "true", + }, + }, + }, + } + }, + desiredRules: func() []gatewayv1beta1.HTTPRouteRule { + rules := routeDemo.DeepCopy().Spec.Rules + iType := gatewayv1beta1.QueryParamMatchRegularExpression + rules = append(rules, gatewayv1beta1.HTTPRouteRule{ + Matches: []gatewayv1beta1.HTTPRouteMatch{ + { + Path: &gatewayv1beta1.HTTPPathMatch{ + Value: utilpointer.String("/store"), + }, + Headers: []gatewayv1beta1.HTTPHeaderMatch{ + { + Name: "version", + Value: "v2", + }, + }, + QueryParams: []gatewayv1beta1.HTTPQueryParamMatch{ + { + Name: "user_id", + Value: "123*", + Type: &iType, + }, + { + Name: "canary", + Value: "true", + }, + }, + }, + { + Path: &gatewayv1beta1.HTTPPathMatch{ + Value: utilpointer.String("/store"), + }, + Headers: []gatewayv1beta1.HTTPHeaderMatch{ + { + Name: "version", + Value: "v2", + }, + }, + QueryParams: []gatewayv1beta1.HTTPQueryParamMatch{ + { + Name: "user_id", + Value: "234*", + Type: &iType, + }, + { + Name: "canary", + Value: "true", + }, + }, + }, + { + Path: &gatewayv1beta1.HTTPPathMatch{ + Value: utilpointer.String("/v2/store"), + }, + QueryParams: []gatewayv1beta1.HTTPQueryParamMatch{ + { + Name: "user_id", + Value: "123*", + Type: &iType, + }, + { + Name: "canary", + Value: "true", + }, + }, + }, + { + Path: &gatewayv1beta1.HTTPPathMatch{ + Value: utilpointer.String("/v2/store"), + }, + QueryParams: []gatewayv1beta1.HTTPQueryParamMatch{ + { + Name: "user_id", + Value: "234*", + Type: &iType, + }, + { + Name: "canary", + Value: "true", + }, + }, + }, + }, + BackendRefs: []gatewayv1beta1.HTTPBackendRef{ + { + BackendRef: gatewayv1beta1.BackendRef{ + BackendObjectReference: gatewayv1beta1.BackendObjectReference{ + Kind: &kindSvc, + Name: "store-svc-canary", + Port: &portNum, + }, + }, + }, + }, + }) + rules = append(rules, gatewayv1beta1.HTTPRouteRule{ + Matches: []gatewayv1beta1.HTTPRouteMatch{ + { + Path: &gatewayv1beta1.HTTPPathMatch{ + Value: utilpointer.String("/storage"), + }, + QueryParams: []gatewayv1beta1.HTTPQueryParamMatch{ + { + Name: "user_id", + Value: "123*", + Type: &iType, + }, + { + Name: "canary", + Value: "true", + }, + }, + }, + { + Path: &gatewayv1beta1.HTTPPathMatch{ + Value: utilpointer.String("/storage"), + }, + QueryParams: []gatewayv1beta1.HTTPQueryParamMatch{ + { + Name: "user_id", + Value: "234*", + Type: &iType, + }, + { + Name: "canary", + Value: "true", + }, + }, + }, + }, + BackendRefs: []gatewayv1beta1.HTTPBackendRef{ + { + BackendRef: gatewayv1beta1.BackendRef{ + BackendObjectReference: gatewayv1beta1.BackendObjectReference{ + Kind: &kindSvc, + Name: "store-svc-canary", + Port: &portNum, + }, + }, + }, + }, + }) + return rules + }, + }, + { + name: "test query params and headers", + getRouteRules: func() []gatewayv1beta1.HTTPRouteRule { + rules := routeDemo.DeepCopy().Spec.Rules + return rules + }, + getRoutes: func() (*int32, []v1beta1.HttpRouteMatch) { + iQueryParamType := gatewayv1beta1.QueryParamMatchRegularExpression + iHeaderType := gatewayv1beta1.HeaderMatchRegularExpression + return nil, []v1beta1.HttpRouteMatch{ + // queryParams + headers + { + QueryParams: []gatewayv1beta1.HTTPQueryParamMatch{ + { + Name: "user_id", + Value: "123*", + Type: &iQueryParamType, + }, + { + Name: "canary", + Value: "true", + }, + }, + Headers: []gatewayv1beta1.HTTPHeaderMatch{ + { + Name: "user_id", + Value: "123*", + Type: &iHeaderType, + }, + { + Name: "canary", + Value: "true", + }, + }, + }, + } + }, + desiredRules: func() []gatewayv1beta1.HTTPRouteRule { + rules := routeDemo.DeepCopy().Spec.Rules + iQueryParamType := gatewayv1beta1.QueryParamMatchRegularExpression + iHeaderType := gatewayv1beta1.HeaderMatchRegularExpression + rules = append(rules, gatewayv1beta1.HTTPRouteRule{ + Matches: []gatewayv1beta1.HTTPRouteMatch{ + { + Path: &gatewayv1beta1.HTTPPathMatch{ + Value: utilpointer.String("/store"), + }, + Headers: []gatewayv1beta1.HTTPHeaderMatch{ + { + Name: "version", + Value: "v2", + }, + { + Name: "user_id", + Value: "123*", + Type: &iHeaderType, + }, + { + Name: "canary", + Value: "true", + }, + }, + QueryParams: []gatewayv1beta1.HTTPQueryParamMatch{ + { + Name: "user_id", + Value: "123*", + Type: &iQueryParamType, + }, + { + Name: "canary", + Value: "true", + }, + }, + }, + { + Path: &gatewayv1beta1.HTTPPathMatch{ + Value: utilpointer.String("/v2/store"), + }, + Headers: []gatewayv1beta1.HTTPHeaderMatch{ + { + Name: "user_id", + Value: "123*", + Type: &iHeaderType, + }, + { + Name: "canary", + Value: "true", + }, + }, + QueryParams: []gatewayv1beta1.HTTPQueryParamMatch{ + { + Name: "user_id", + Value: "123*", + Type: &iQueryParamType, + }, + { + Name: "canary", + Value: "true", + }, + }, + }, + }, + BackendRefs: []gatewayv1beta1.HTTPBackendRef{ + { + BackendRef: gatewayv1beta1.BackendRef{ + BackendObjectReference: gatewayv1beta1.BackendObjectReference{ + Kind: &kindSvc, + Name: "store-svc-canary", + Port: &portNum, + }, + }, + }, + }, + }) + rules = append(rules, gatewayv1beta1.HTTPRouteRule{ + Matches: []gatewayv1beta1.HTTPRouteMatch{ + { + Path: &gatewayv1beta1.HTTPPathMatch{ + Value: utilpointer.String("/storage"), + }, + Headers: []gatewayv1beta1.HTTPHeaderMatch{ + { + Name: "user_id", + Value: "123*", + Type: &iHeaderType, + }, + { + Name: "canary", + Value: "true", + }, + }, + QueryParams: []gatewayv1beta1.HTTPQueryParamMatch{ + { + Name: "user_id", + Value: "123*", + Type: &iQueryParamType, + }, + { + Name: "canary", + Value: "true", + }, + }, + }, + }, + BackendRefs: []gatewayv1beta1.HTTPBackendRef{ + { + BackendRef: gatewayv1beta1.BackendRef{ + BackendObjectReference: gatewayv1beta1.BackendObjectReference{ + Kind: &kindSvc, + Name: "store-svc-canary", + Port: &portNum, + }, + }, + }, + }, + }) + return rules + }, + }, + { + name: "test path replace", + getRouteRules: func() []gatewayv1beta1.HTTPRouteRule { + rules := routeDemo.DeepCopy().Spec.Rules + return rules + }, + getRoutes: func() (*int32, []v1beta1.HttpRouteMatch) { + iQueryParamType := gatewayv1beta1.QueryParamMatchRegularExpression + iHeaderType := gatewayv1beta1.HeaderMatchRegularExpression + return nil, []v1beta1.HttpRouteMatch{ + // queryParams + headers + path + { + QueryParams: []gatewayv1beta1.HTTPQueryParamMatch{ + { + Name: "user_id", + Value: "123*", + Type: &iQueryParamType, + }, + { + Name: "canary", + Value: "true", + }, + }, + Headers: []gatewayv1beta1.HTTPHeaderMatch{ + { + Name: "user_id", + Value: "123*", + Type: &iHeaderType, + }, + { + Name: "canary", + Value: "true", + }, + }, + Path: &gatewayv1beta1.HTTPPathMatch{ + Value: utilpointer.String("/storage/v2"), + }, + }, + } + }, + desiredRules: func() []gatewayv1beta1.HTTPRouteRule { + rules := routeDemo.DeepCopy().Spec.Rules + iQueryParamType := gatewayv1beta1.QueryParamMatchRegularExpression + iHeaderType := gatewayv1beta1.HeaderMatchRegularExpression + rules = append(rules, gatewayv1beta1.HTTPRouteRule{ + Matches: []gatewayv1beta1.HTTPRouteMatch{ + { + Path: &gatewayv1beta1.HTTPPathMatch{ + Value: utilpointer.String("/storage/v2"), + }, + Headers: []gatewayv1beta1.HTTPHeaderMatch{ + { + Name: "user_id", + Value: "123*", + Type: &iHeaderType, + }, + { + Name: "canary", + Value: "true", + }, + }, + QueryParams: []gatewayv1beta1.HTTPQueryParamMatch{ + { + Name: "user_id", + Value: "123*", + Type: &iQueryParamType, + }, + { + Name: "canary", + Value: "true", + }, + }, + }, + }, + BackendRefs: []gatewayv1beta1.HTTPBackendRef{ + { + BackendRef: gatewayv1beta1.BackendRef{ + BackendObjectReference: gatewayv1beta1.BackendObjectReference{ + Kind: &kindSvc, + Name: "store-svc-canary", + Port: &portNum, + }, + }, + }, + }, + }) + return rules + }, + }, { name: "canary weight: 20", getRouteRules: func() []gatewayv1beta1.HTTPRouteRule { diff --git a/pkg/trafficrouting/network/ingress/ingress_test.go b/pkg/trafficrouting/network/ingress/ingress_test.go index a2dcd56f..23aada3d 100644 --- a/pkg/trafficrouting/network/ingress/ingress_test.go +++ b/pkg/trafficrouting/network/ingress/ingress_test.go @@ -57,6 +57,10 @@ var ( annotations["nginx.ingress.kubernetes.io/canary-by-header"] = nil annotations["nginx.ingress.kubernetes.io/canary-by-header-pattern"] = nil annotations["nginx.ingress.kubernetes.io/canary-by-header-value"] = nil + -- MSE extended annotations + annotations["mse.ingress.kubernetes.io/canary-by-query"] = nil + annotations["mse.ingress.kubernetes.io/canary-by-query-pattern"] = nil + annotations["mse.ingress.kubernetes.io/canary-by-query-value"] = nil annotations["nginx.ingress.kubernetes.io/canary-weight"] = nil if ( obj.weight ~= "-1" ) then @@ -79,17 +83,29 @@ var ( return annotations end for _,match in ipairs(obj.matches) do - header = match.headers[1] - if ( header.name == "canary-by-cookie" ) - then - annotations["nginx.ingress.kubernetes.io/canary-by-cookie"] = header.value - else - annotations["nginx.ingress.kubernetes.io/canary-by-header"] = header.name - if ( header.type == "RegularExpression" ) + if match.headers and next(match.headers) ~= nil then + header = match.headers[1] + if ( header.name == "canary-by-cookie" ) + then + annotations["nginx.ingress.kubernetes.io/canary-by-cookie"] = header.value + else + annotations["nginx.ingress.kubernetes.io/canary-by-header"] = header.name + if ( header.type == "RegularExpression" ) + then + annotations["nginx.ingress.kubernetes.io/canary-by-header-pattern"] = header.value + else + annotations["nginx.ingress.kubernetes.io/canary-by-header-value"] = header.value + end + end + end + if match.queryParams and next(match.queryParams) ~= nil then + queryParam = match.queryParams[1] + annotations["nginx.ingress.kubernetes.io/canary-by-query"] = queryParam.name + if ( queryParam.type == "RegularExpression" ) then - annotations["nginx.ingress.kubernetes.io/canary-by-header-pattern"] = header.value + annotations["nginx.ingress.kubernetes.io/canary-by-query-pattern"] = queryParam.value else - annotations["nginx.ingress.kubernetes.io/canary-by-header-value"] = header.value + annotations["nginx.ingress.kubernetes.io/canary-by-query-value"] = queryParam.value end end end @@ -519,6 +535,128 @@ func TestEnsureRoutes(t *testing.T) { return expect }, }, + { + name: "ensure routes test5", + getConfigmap: func() *corev1.ConfigMap { + return demoConf.DeepCopy() + }, + getIngress: func() []*netv1.Ingress { + canary := demoIngress.DeepCopy() + canary.Name = "echoserver-canary" + canary.Annotations["nginx.ingress.kubernetes.io/canary"] = "true" + canary.Annotations["nginx.ingress.kubernetes.io/canary-weight"] = "0" + canary.Annotations["mse.ingress.kubernetes.io/service-subset"] = "" + canary.Spec.Rules[0].HTTP.Paths = canary.Spec.Rules[0].HTTP.Paths[:1] + canary.Spec.Rules[0].HTTP.Paths[0].Backend.Service.Name = "echoserver-canary" + canary.Spec.Rules[1].HTTP.Paths[0].Backend.Service.Name = "echoserver-canary" + return []*netv1.Ingress{demoIngress.DeepCopy(), canary} + }, + getRoutes: func() *v1beta1.CanaryStep { + return &v1beta1.CanaryStep{ + TrafficRoutingStrategy: v1beta1.TrafficRoutingStrategy{ + Traffic: nil, + Matches: []v1beta1.HttpRouteMatch{ + // querystring + { + QueryParams: []gatewayv1beta1.HTTPQueryParamMatch{ + { + Name: "user_id", + Value: "123456", + }, + }, + }, + }, + RequestHeaderModifier: &gatewayv1beta1.HTTPRequestHeaderFilter{ + Set: []gatewayv1beta1.HTTPHeader{ + { + Name: "gray", + Value: "blue", + }, + { + Name: "gray", + Value: "green", + }, + }, + }, + }, + } + }, + expectIngress: func() *netv1.Ingress { + expect := demoIngress.DeepCopy() + expect.Name = "echoserver-canary" + expect.Annotations["nginx.ingress.kubernetes.io/canary"] = "true" + expect.Annotations["nginx.ingress.kubernetes.io/canary-by-query"] = "user_id" + expect.Annotations["nginx.ingress.kubernetes.io/canary-by-query-value"] = "123456" + expect.Annotations["mse.ingress.kubernetes.io/request-header-control-update"] = "gray blue\ngray green\n" + expect.Annotations["mse.ingress.kubernetes.io/service-subset"] = "gray" + expect.Spec.Rules[0].HTTP.Paths = expect.Spec.Rules[0].HTTP.Paths[:1] + expect.Spec.Rules[0].HTTP.Paths[0].Backend.Service.Name = "echoserver-canary" + expect.Spec.Rules[1].HTTP.Paths[0].Backend.Service.Name = "echoserver-canary" + return expect + }, + }, + { + name: "ensure routes test5", + getConfigmap: func() *corev1.ConfigMap { + return demoConf.DeepCopy() + }, + getIngress: func() []*netv1.Ingress { + canary := demoIngress.DeepCopy() + canary.Name = "echoserver-canary" + canary.Annotations["nginx.ingress.kubernetes.io/canary"] = "true" + canary.Annotations["nginx.ingress.kubernetes.io/canary-weight"] = "0" + canary.Annotations["mse.ingress.kubernetes.io/service-subset"] = "" + canary.Spec.Rules[0].HTTP.Paths = canary.Spec.Rules[0].HTTP.Paths[:1] + canary.Spec.Rules[0].HTTP.Paths[0].Backend.Service.Name = "echoserver-canary" + canary.Spec.Rules[1].HTTP.Paths[0].Backend.Service.Name = "echoserver-canary" + return []*netv1.Ingress{demoIngress.DeepCopy(), canary} + }, + getRoutes: func() *v1beta1.CanaryStep { + iType := gatewayv1beta1.QueryParamMatchRegularExpression + return &v1beta1.CanaryStep{ + TrafficRoutingStrategy: v1beta1.TrafficRoutingStrategy{ + Traffic: nil, + Matches: []v1beta1.HttpRouteMatch{ + // querystring + { + QueryParams: []gatewayv1beta1.HTTPQueryParamMatch{ + { + Name: "user_id", + Value: "123*", + Type: &iType, + }, + }, + }, + }, + RequestHeaderModifier: &gatewayv1beta1.HTTPRequestHeaderFilter{ + Set: []gatewayv1beta1.HTTPHeader{ + { + Name: "gray", + Value: "blue", + }, + { + Name: "gray", + Value: "green", + }, + }, + }, + }, + } + }, + expectIngress: func() *netv1.Ingress { + expect := demoIngress.DeepCopy() + expect.Name = "echoserver-canary" + expect.Annotations["nginx.ingress.kubernetes.io/canary"] = "true" + expect.Annotations["nginx.ingress.kubernetes.io/canary-by-query"] = "user_id" + expect.Annotations["nginx.ingress.kubernetes.io/canary-by-query-pattern"] = "123*" + expect.Annotations["mse.ingress.kubernetes.io/request-header-control-update"] = "gray blue\ngray green\n" + expect.Annotations["mse.ingress.kubernetes.io/service-subset"] = "gray" + expect.Spec.Rules[0].HTTP.Paths = expect.Spec.Rules[0].HTTP.Paths[:1] + expect.Spec.Rules[0].HTTP.Paths[0].Backend.Service.Name = "echoserver-canary" + expect.Spec.Rules[1].HTTP.Paths[0].Backend.Service.Name = "echoserver-canary" + return expect + }, + }, } config := Config{ diff --git a/pkg/util/slices_utils.go b/pkg/util/slices_utils.go new file mode 100644 index 00000000..5a8b83fd --- /dev/null +++ b/pkg/util/slices_utils.go @@ -0,0 +1,28 @@ +/* +Copyright 2022 The Kruise Authors. +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + http://www.apache.org/licenses/LICENSE-2.0 +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package util + +import ( + "github.com/openkruise/rollouts/api/v1beta1" +) + +func FilterHttpRouteMatch(s []v1beta1.HttpRouteMatch, f func(v1beta1.HttpRouteMatch) bool) []v1beta1.HttpRouteMatch { + s2 := make([]v1beta1.HttpRouteMatch, 0, len(s)) + for _, e := range s { + if f(e) { + s2 = append(s2, e) + } + } + return s2 +} diff --git a/test/e2e/rollout_test.go b/test/e2e/rollout_test.go index c8a7cae2..caea11bb 100644 --- a/test/e2e/rollout_test.go +++ b/test/e2e/rollout_test.go @@ -28,6 +28,7 @@ import ( appsv1alpha1 "github.com/openkruise/kruise-api/apps/v1alpha1" appsv1beta1 "github.com/openkruise/kruise-api/apps/v1beta1" "github.com/openkruise/rollouts/api/v1alpha1" + "github.com/openkruise/rollouts/api/v1beta1" "github.com/openkruise/rollouts/pkg/util" apps "k8s.io/api/apps/v1" v1 "k8s.io/api/core/v1" @@ -59,6 +60,16 @@ func getRolloutCondition(status v1alpha1.RolloutStatus, condType v1alpha1.Rollou return nil } +func getRolloutConditionV1beta1(status v1beta1.RolloutStatus, condType v1beta1.RolloutConditionType) *v1beta1.RolloutCondition { + for i := range status.Conditions { + c := status.Conditions[i] + if c.Type == condType { + return &c + } + } + return nil +} + var _ = SIGDescribe("Rollout", func() { var namespace string @@ -218,6 +229,21 @@ var _ = SIGDescribe("Rollout", func() { }, 10*time.Second, time.Second).Should(BeTrue()) } + ResumeRolloutCanaryV1beta1 := func(name string) { + Eventually(func() bool { + clone := &v1beta1.Rollout{} + Expect(GetObject(name, clone)).NotTo(HaveOccurred()) + if clone.Status.CanaryStatus.CurrentStepState != v1beta1.CanaryStepStatePaused { + fmt.Println("resume rollout success, and CurrentStepState", util.DumpJSON(clone.Status)) + return true + } + + body := fmt.Sprintf(`{"status":{"canaryStatus":{"currentStepState":"%s"}}}`, v1beta1.CanaryStepStateReady) + Expect(k8sClient.Status().Patch(context.TODO(), clone, client.RawPatch(types.MergePatchType, []byte(body)))).NotTo(HaveOccurred()) + return false + }, 10*time.Second, time.Second).Should(BeTrue()) + } + WaitDeploymentAllPodsReady := func(deployment *apps.Deployment) { Eventually(func() bool { clone := &apps.Deployment{} @@ -2652,9 +2678,9 @@ var _ = SIGDescribe("Rollout", func() { }) KruiseDescribe("Canary rollout with custon network provider", func() { - It("V1->V2: Route traffic with header matches and weight using rollout for VirtualService", func() { + It("V1->V2: Route traffic with header/queryParams/path matches and weight using rollout for VirtualService", func() { By("Creating Rollout...") - rollout := &v1alpha1.Rollout{} + rollout := &v1beta1.Rollout{} Expect(ReadYamlToObject("./test_data/customNetworkProvider/rollout_with_trafficrouting.yaml", rollout)).ToNot(HaveOccurred()) CreateObject(rollout) @@ -2678,7 +2704,7 @@ var _ = SIGDescribe("Rollout", func() { // check rollout status Expect(GetObject(rollout.Name, rollout)).NotTo(HaveOccurred()) - Expect(rollout.Status.Phase).Should(Equal(v1alpha1.RolloutPhaseHealthy)) + Expect(rollout.Status.Phase).Should(Equal(v1beta1.RolloutPhaseHealthy)) By("check rollout status & paused success") // v1 -> v2, start rollout action @@ -2698,14 +2724,14 @@ var _ = SIGDescribe("Rollout", func() { Expect(rollout.Status.CanaryStatus.CanaryReadyReplicas).Should(BeNumerically("==", 1)) // check virtualservice spec Expect(GetObject(vs.GetName(), vs)).NotTo(HaveOccurred()) - expectedSpec := `{"gateways":["nginx-gateway"],"hosts":["*"],"http":[{"match":[{"headers":{"user-agent":{"exact":"pc"}}}],"route":[{"destination":{"host":"echoserver-canary"}}]},{"route":[{"destination":{"host":"echoserver"}}]}]}` + expectedSpec := `{"gateways":["nginx-gateway"],"hosts":["*"],"http":[{"match":[{"uri":{"prefix":"/pc"}}],"route":[{"destination":{"host":"echoserver-canary"}}]},{"match":[{"queryParams":{"user-agent":{"exact":"pc"}}}],"route":[{"destination":{"host":"echoserver-canary"}}]},{"match":[{"headers":{"user-agent":{"exact":"pc"}}}],"route":[{"destination":{"host":"echoserver-canary"}}]},{"route":[{"destination":{"host":"echoserver"}}]}]}` Expect(util.DumpJSON(vs.Object["spec"])).Should(Equal(expectedSpec)) // check original spec annotation expectedAnno := `{"spec":{"gateways":["nginx-gateway"],"hosts":["*"],"http":[{"route":[{"destination":{"host":"echoserver"}}]}]}}` Expect(vs.GetAnnotations()[OriginalSpecAnnotation]).Should(Equal(expectedAnno)) // resume rollout canary - ResumeRolloutCanary(rollout.Name) + ResumeRolloutCanaryV1beta1(rollout.Name) By("Resume rollout, and wait next step(2), routing 50% traffic to new version pods") WaitRolloutCanaryStepPaused(rollout.Name, 2) // check rollout status @@ -2720,7 +2746,7 @@ var _ = SIGDescribe("Rollout", func() { Expect(vs.GetAnnotations()[OriginalSpecAnnotation]).Should(Equal(expectedAnno)) // resume rollout - ResumeRolloutCanary(rollout.Name) + ResumeRolloutCanaryV1beta1(rollout.Name) WaitRolloutStatusPhase(rollout.Name, v1alpha1.RolloutPhaseHealthy) By("rollout completed, and check") // check service & virtualservice & deployment @@ -2747,10 +2773,10 @@ var _ = SIGDescribe("Rollout", func() { } // check progressing succeed Expect(GetObject(rollout.Name, rollout)).NotTo(HaveOccurred()) - cond := getRolloutCondition(rollout.Status, v1alpha1.RolloutConditionProgressing) - Expect(cond.Reason).Should(Equal(v1alpha1.ProgressingReasonCompleted)) + cond := getRolloutConditionV1beta1(rollout.Status, v1beta1.RolloutConditionProgressing) + Expect(cond.Reason).Should(Equal(v1beta1.ProgressingReasonCompleted)) Expect(string(cond.Status)).Should(Equal(string(metav1.ConditionFalse))) - cond = getRolloutCondition(rollout.Status, v1alpha1.RolloutConditionSucceeded) + cond = getRolloutConditionV1beta1(rollout.Status, v1beta1.RolloutConditionSucceeded) Expect(string(cond.Status)).Should(Equal(string(metav1.ConditionTrue))) Expect(GetObject(workload.Name, workload)).NotTo(HaveOccurred()) WaitRolloutWorkloadGeneration(rollout.Name, workload.Generation) diff --git a/test/e2e/test_data/customNetworkProvider/rollout_with_trafficrouting.yaml b/test/e2e/test_data/customNetworkProvider/rollout_with_trafficrouting.yaml index f96a0f92..713ce5df 100644 --- a/test/e2e/test_data/customNetworkProvider/rollout_with_trafficrouting.yaml +++ b/test/e2e/test_data/customNetworkProvider/rollout_with_trafficrouting.yaml @@ -1,18 +1,16 @@ - apiVersion: rollouts.kruise.io/v1alpha1 + apiVersion: rollouts.kruise.io/v1beta1 kind: Rollout metadata: name: rollouts-demo - annotations: - rollouts.kruise.io/rolling-style: canary spec: disabled: false - objectRef: - workloadRef: - apiVersion: apps/v1 - kind: Deployment - name: echoserver + workloadRef: + apiVersion: apps/v1 + kind: Deployment + name: echoserver strategy: canary: + enableExtraWorkloadForCanary: true steps: - replicas: 1 matches: @@ -20,7 +18,14 @@ - type: Exact name: user-agent value: pc - - weight: 50 + - queryParams: + - type: Exact + name: user-agent + value: pc + - path: + value: /pc + - replicas: "50%" + traffic: "50%" trafficRoutings: - service: echoserver customNetworkRefs: From 1e8af4a4c177bc77b1b6842c2730d1a84beb22eb Mon Sep 17 00:00:00 2001 From: myname4423 <57184070+myname4423@users.noreply.github.com> Date: Wed, 12 Jun 2024 13:27:41 +0800 Subject: [PATCH 3/3] update api for future bluegreen (#214) add status conversion nextStepIndex default value from 0 to -1 restore the enableExtra field in BR Signed-off-by: yunbo Co-authored-by: yunbo --- api/v1alpha1/batchrelease_plan_types.go | 10 + api/v1alpha1/conversion.go | 55 ++- api/v1alpha1/deployment_types.go | 4 +- api/v1alpha1/rollout_types.go | 24 +- api/v1beta1/batchrelease_plan_types.go | 12 +- api/v1beta1/deployment_types.go | 80 +++- api/v1beta1/rollout_types.go | 254 ++++++++++- api/v1beta1/zz_generated.deepcopy.go | 109 ++++- .../rollouts.kruise.io_batchreleases.yaml | 27 +- .../bases/rollouts.kruise.io_rollouts.yaml | 418 +++++++++++++++++- .../batchrelease_controller_test.go | 5 +- .../batchrelease/batchrelease_executor.go | 43 +- pkg/controller/rollout/rollout_canary.go | 1 + pkg/controller/rollout/rollout_canary_test.go | 9 +- pkg/controller/rollout/rollout_progressing.go | 22 +- .../rollout/rollout_progressing_test.go | 2 + pkg/controller/rollout/rollout_status.go | 19 +- pkg/trafficrouting/manager_test.go | 40 +- pkg/util/rollout_utils.go | 12 + .../rollout_create_update_handler.go | 68 ++- .../rollout_create_update_handler_test.go | 4 +- test/e2e/deployment_test.go | 21 + test/e2e/rollout_test.go | 19 +- 23 files changed, 1136 insertions(+), 122 deletions(-) diff --git a/api/v1alpha1/batchrelease_plan_types.go b/api/v1alpha1/batchrelease_plan_types.go index 0cf37e8c..01dda464 100644 --- a/api/v1alpha1/batchrelease_plan_types.go +++ b/api/v1alpha1/batchrelease_plan_types.go @@ -54,6 +54,16 @@ type ReleasePlan struct { // only support for canary deployment // +optional PatchPodTemplateMetadata *PatchPodTemplateMetadata `json:"patchPodTemplateMetadata,omitempty"` + // RollingStyle can be "Canary", "Partiton" or "BlueGreen" + RollingStyle RollingStyleType `json:"rollingStyle,omitempty"` + // EnableExtraWorkloadForCanary indicates whether to create extra workload for canary + // True corresponds to RollingStyle "Canary". + // False corresponds to RollingStyle "Partiton". + // Ignored in BlueGreen-style. + // This field is about to deprecate, use RollingStyle instead. + // If both of them are set, controller will only consider this + // filed when RollingStyle is empty + EnableExtraWorkloadForCanary bool `json:"enableExtraWorkloadForCanary"` } type FinalizingPolicyType string diff --git a/api/v1alpha1/conversion.go b/api/v1alpha1/conversion.go index 5164c1ca..28ad4a9c 100644 --- a/api/v1alpha1/conversion.go +++ b/api/v1alpha1/conversion.go @@ -104,18 +104,22 @@ func (src *Rollout) ConvertTo(dst conversion.Hub) error { return nil } obj.Status.CanaryStatus = &v1beta1.CanaryStatus{ - ObservedWorkloadGeneration: src.Status.CanaryStatus.ObservedWorkloadGeneration, - ObservedRolloutID: src.Status.CanaryStatus.ObservedRolloutID, - RolloutHash: src.Status.CanaryStatus.RolloutHash, - StableRevision: src.Status.CanaryStatus.StableRevision, - CanaryRevision: src.Status.CanaryStatus.CanaryRevision, - PodTemplateHash: src.Status.CanaryStatus.PodTemplateHash, - CanaryReplicas: src.Status.CanaryStatus.CanaryReplicas, - CanaryReadyReplicas: src.Status.CanaryStatus.CanaryReadyReplicas, - CurrentStepIndex: src.Status.CanaryStatus.CurrentStepIndex, - CurrentStepState: v1beta1.CanaryStepState(src.Status.CanaryStatus.CurrentStepState), - Message: src.Status.CanaryStatus.Message, - LastUpdateTime: src.Status.CanaryStatus.LastUpdateTime, + CommonStatus: v1beta1.CommonStatus{ + ObservedWorkloadGeneration: src.Status.CanaryStatus.ObservedWorkloadGeneration, + ObservedRolloutID: src.Status.CanaryStatus.ObservedRolloutID, + RolloutHash: src.Status.CanaryStatus.RolloutHash, + StableRevision: src.Status.CanaryStatus.StableRevision, + PodTemplateHash: src.Status.CanaryStatus.PodTemplateHash, + CurrentStepIndex: src.Status.CanaryStatus.CurrentStepIndex, + CurrentStepState: v1beta1.CanaryStepState(src.Status.CanaryStatus.CurrentStepState), + Message: src.Status.CanaryStatus.Message, + LastUpdateTime: src.Status.CanaryStatus.LastUpdateTime, + FinalisingStep: v1beta1.FinalisingStepType(src.Status.CanaryStatus.FinalisingStep), + NextStepIndex: src.Status.CanaryStatus.NextStepIndex, + }, + CanaryRevision: src.Status.CanaryStatus.CanaryRevision, + CanaryReplicas: src.Status.CanaryStatus.CanaryReplicas, + CanaryReadyReplicas: src.Status.CanaryStatus.CanaryReadyReplicas, } return nil default: @@ -167,7 +171,9 @@ func (dst *Rollout) ConvertFrom(src conversion.Hub) error { case *v1beta1.Rollout: srcV1beta1 := src.(*v1beta1.Rollout) dst.ObjectMeta = srcV1beta1.ObjectMeta - + if !srcV1beta1.Spec.Strategy.IsCanaryStragegy() { + return fmt.Errorf("v1beta1 Rollout with %s strategy cannot be converted to v1alpha1", srcV1beta1.Spec.Strategy.GetRollingStyle()) + } // spec dst.Spec = RolloutSpec{ ObjectRef: ObjectRef{ @@ -254,6 +260,8 @@ func (dst *Rollout) ConvertFrom(src conversion.Hub) error { CurrentStepState: CanaryStepState(srcV1beta1.Status.CanaryStatus.CurrentStepState), Message: srcV1beta1.Status.CanaryStatus.Message, LastUpdateTime: srcV1beta1.Status.CanaryStatus.LastUpdateTime, + FinalisingStep: FinalizeStateType(srcV1beta1.Status.CanaryStatus.FinalisingStep), + NextStepIndex: srcV1beta1.Status.CanaryStatus.NextStepIndex, } return nil default: @@ -338,9 +346,18 @@ func (src *BatchRelease) ConvertTo(dst conversion.Hub) error { obj.Spec.ReleasePlan.PatchPodTemplateMetadata.Labels[k] = v } } - if !strings.EqualFold(src.Annotations[RolloutStyleAnnotation], string(PartitionRollingStyle)) { - obj.Spec.ReleasePlan.EnableExtraWorkloadForCanary = true + + if strings.EqualFold(src.Annotations[RolloutStyleAnnotation], string(PartitionRollingStyle)) { + obj.Spec.ReleasePlan.RollingStyle = v1beta1.PartitionRollingStyle + } + if strings.EqualFold(src.Annotations[RolloutStyleAnnotation], string(CanaryRollingStyle)) { + obj.Spec.ReleasePlan.RollingStyle = v1beta1.CanaryRollingStyle } + if strings.EqualFold(src.Annotations[RolloutStyleAnnotation], string(BlueGreenRollingStyle)) { + obj.Spec.ReleasePlan.RollingStyle = v1beta1.BlueGreenRollingStyle + } + + obj.Spec.ReleasePlan.EnableExtraWorkloadForCanary = srcSpec.ReleasePlan.EnableExtraWorkloadForCanary // status obj.Status = v1beta1.BatchReleaseStatus{ @@ -417,11 +434,9 @@ func (dst *BatchRelease) ConvertFrom(src conversion.Hub) error { if dst.Annotations == nil { dst.Annotations = map[string]string{} } - if srcV1beta1.Spec.ReleasePlan.EnableExtraWorkloadForCanary { - dst.Annotations[RolloutStyleAnnotation] = strings.ToLower(string(CanaryRollingStyle)) - } else { - dst.Annotations[RolloutStyleAnnotation] = strings.ToLower(string(PartitionRollingStyle)) - } + dst.Annotations[RolloutStyleAnnotation] = strings.ToLower(string(srcV1beta1.Spec.ReleasePlan.RollingStyle)) + dst.Spec.ReleasePlan.RollingStyle = RollingStyleType(srcV1beta1.Spec.ReleasePlan.RollingStyle) + dst.Spec.ReleasePlan.EnableExtraWorkloadForCanary = srcV1beta1.Spec.ReleasePlan.EnableExtraWorkloadForCanary // status dst.Status = BatchReleaseStatus{ diff --git a/api/v1alpha1/deployment_types.go b/api/v1alpha1/deployment_types.go index 35202e5b..98fc474c 100644 --- a/api/v1alpha1/deployment_types.go +++ b/api/v1alpha1/deployment_types.go @@ -59,6 +59,8 @@ const ( PartitionRollingStyle RollingStyleType = "Partition" // CanaryRollingStyle means rolling in canary way, and will create a canary Deployment. CanaryRollingStyle RollingStyleType = "Canary" + // BlueGreenRollingStyle means rolling in blue-green way, and will NOT create a canary Deployment. + BlueGreenRollingStyle RollingStyleType = "BlueGreen" ) // DeploymentExtraStatus is extra status field for Advanced Deployment @@ -74,7 +76,7 @@ type DeploymentExtraStatus struct { } func SetDefaultDeploymentStrategy(strategy *DeploymentStrategy) { - if strategy.RollingStyle == CanaryRollingStyle { + if strategy.RollingStyle != PartitionRollingStyle { return } if strategy.RollingUpdate == nil { diff --git a/api/v1alpha1/rollout_types.go b/api/v1alpha1/rollout_types.go index 697287d8..d67a5388 100644 --- a/api/v1alpha1/rollout_types.go +++ b/api/v1alpha1/rollout_types.go @@ -242,16 +242,28 @@ type CanaryStatus struct { CanaryReplicas int32 `json:"canaryReplicas"` // CanaryReadyReplicas the numbers of ready canary revision pods CanaryReadyReplicas int32 `json:"canaryReadyReplicas"` - // CurrentStepIndex defines the current step of the rollout is on. If the current step index is null, the - // controller will execute the rollout. + // NextStepIndex defines the next step of the rollout is on. + // In normal case, NextStepIndex is equal to CurrentStepIndex + 1 + // If the current step is the last step, NextStepIndex is equal to -1 + // Before the release, NextStepIndex is also equal to -1 + // 0 is not used and won't appear in any case + // It is allowed to patch NextStepIndex by design, + // e.g. if CurrentStepIndex is 2, user can patch NextStepIndex to 3 (if exists) to + // achieve batch jump, or patch NextStepIndex to 1 to implement a re-execution of step 1 + // Patching it with a non-positive value is meaningless, which will be corrected + // in the next reconciliation + // achieve batch jump, or patch NextStepIndex to 1 to implement a re-execution of step 1 + NextStepIndex int32 `json:"nextStepIndex"` // +optional - CurrentStepIndex int32 `json:"currentStepIndex"` - CurrentStepState CanaryStepState `json:"currentStepState"` - Message string `json:"message,omitempty"` - LastUpdateTime *metav1.Time `json:"lastUpdateTime,omitempty"` + CurrentStepIndex int32 `json:"currentStepIndex"` + CurrentStepState CanaryStepState `json:"currentStepState"` + Message string `json:"message,omitempty"` + LastUpdateTime *metav1.Time `json:"lastUpdateTime,omitempty"` + FinalisingStep FinalizeStateType `json:"finalisingStep"` } type CanaryStepState string +type FinalizeStateType string const ( CanaryStepStateUpgrade CanaryStepState = "StepUpgrade" diff --git a/api/v1beta1/batchrelease_plan_types.go b/api/v1beta1/batchrelease_plan_types.go index 8b3c21a2..ce7f6709 100644 --- a/api/v1beta1/batchrelease_plan_types.go +++ b/api/v1beta1/batchrelease_plan_types.go @@ -54,9 +54,15 @@ type ReleasePlan struct { // only support for canary deployment // +optional PatchPodTemplateMetadata *PatchPodTemplateMetadata `json:"patchPodTemplateMetadata,omitempty"` - // If true, then it will create new deployment for canary, such as: workload-demo-canary. - // When user verifies that the canary version is ready, we will remove the canary deployment and release the deployment workload-demo in full. - // Current only support k8s native deployment + // RollingStyle can be "Canary", "Partiton" or "BlueGreen" + RollingStyle RollingStyleType `json:"rollingStyle,omitempty"` + // EnableExtraWorkloadForCanary indicates whether to create extra workload for canary + // True corresponds to RollingStyle "Canary". + // False corresponds to RollingStyle "Partiton". + // Ignored in BlueGreen-style. + // This field is about to deprecate, use RollingStyle instead. + // If both of them are set, controller will only consider this + // filed when RollingStyle is empty EnableExtraWorkloadForCanary bool `json:"enableExtraWorkloadForCanary"` } diff --git a/api/v1beta1/deployment_types.go b/api/v1beta1/deployment_types.go index 3db56143..9975e989 100644 --- a/api/v1beta1/deployment_types.go +++ b/api/v1beta1/deployment_types.go @@ -37,6 +37,16 @@ const ( // AdvancedDeploymentControlLabel is label for deployment, // which labels whether the deployment is controlled by advanced-deployment-controller. AdvancedDeploymentControlLabel = "rollouts.kruise.io/controlled-by-advanced-deployment-controller" + + // OriginalDeploymentStrategyAnnotation is annotation for workload in BlueGreen Release, + // it will store the original setting of the workload, which will be used to restore the workload + OriginalDeploymentStrategyAnnotation = "rollouts.kruise.io/original-deployment-strategy" + + // MaxProgressSeconds is the value we set for ProgressDeadlineSeconds + // MaxReadySeconds is the value we set for MinReadySeconds, which is one less than ProgressDeadlineSeconds + // MaxInt32: 2147483647, ≈ 68 years + MaxProgressSeconds = 1<<31 - 1 + MaxReadySeconds = MaxProgressSeconds - 1 ) // DeploymentStrategy is strategy field for Advanced Deployment @@ -52,6 +62,31 @@ type DeploymentStrategy struct { Partition intstr.IntOrString `json:"partition,omitempty"` } +// OriginalDeploymentStrategy stores part of the fileds of a workload, +// so that it can be restored when finalizing. +// It is only used for BlueGreen Release +// Similar to DeploymentStrategy, it is an annotation used in workload +// However, unlike DeploymentStrategy, it is only used to store and restore the user's strategy +type OriginalDeploymentStrategy struct { + // The deployment strategy to use to replace existing pods with new ones. + // +optional + // +patchStrategy=retainKeys + Strategy *apps.DeploymentStrategy `json:"strategy,omitempty" patchStrategy:"retainKeys" protobuf:"bytes,4,opt,name=strategy"` + + // Minimum number of seconds for which a newly created pod should be ready + // without any of its container crashing, for it to be considered available. + // Defaults to 0 (pod will be considered available as soon as it is ready) + // +optional + MinReadySeconds int32 `json:"minReadySeconds,omitempty" protobuf:"varint,5,opt,name=minReadySeconds"` + + // The maximum time in seconds for a deployment to make progress before it + // is considered to be failed. The deployment controller will continue to + // process failed deployments and a condition with a ProgressDeadlineExceeded + // reason will be surfaced in the deployment status. Note that progress will + // not be estimated during the time a deployment is paused. Defaults to 600s. + ProgressDeadlineSeconds *int32 `json:"progressDeadlineSeconds,omitempty" protobuf:"varint,9,opt,name=progressDeadlineSeconds"` +} + type RollingStyleType string const ( @@ -59,6 +94,8 @@ const ( PartitionRollingStyle RollingStyleType = "Partition" // CanaryRollingStyle means rolling in canary way, and will create a canary Deployment. CanaryRollingStyle RollingStyleType = "Canary" + // BlueGreenRollingStyle means rolling in blue-green way, and will NOT create a extra Deployment. + BlueGreenRollingStyle RollingStyleType = "BlueGreen" ) // DeploymentExtraStatus is extra status field for Advanced Deployment @@ -74,7 +111,7 @@ type DeploymentExtraStatus struct { } func SetDefaultDeploymentStrategy(strategy *DeploymentStrategy) { - if strategy.RollingStyle == CanaryRollingStyle { + if strategy.RollingStyle != PartitionRollingStyle { return } if strategy.RollingUpdate == nil { @@ -101,3 +138,44 @@ func SetDefaultDeploymentStrategy(strategy *DeploymentStrategy) { } } } + +func SetDefaultSetting(setting *OriginalDeploymentStrategy) { + if setting.ProgressDeadlineSeconds == nil { + setting.ProgressDeadlineSeconds = new(int32) + *setting.ProgressDeadlineSeconds = 600 + } + if setting.Strategy == nil { + setting.Strategy = &apps.DeploymentStrategy{} + } + if setting.Strategy.Type == "" { + setting.Strategy.Type = apps.RollingUpdateDeploymentStrategyType + } + if setting.Strategy.Type == apps.RecreateDeploymentStrategyType { + return + } + strategy := setting.Strategy + if strategy.RollingUpdate == nil { + strategy.RollingUpdate = &apps.RollingUpdateDeployment{} + } + if strategy.RollingUpdate.MaxUnavailable == nil { + // Set MaxUnavailable as 25% by default + maxUnavailable := intstr.FromString("25%") + strategy.RollingUpdate.MaxUnavailable = &maxUnavailable + } + if strategy.RollingUpdate.MaxSurge == nil { + // Set MaxSurge as 25% by default + maxSurge := intstr.FromString("25%") + strategy.RollingUpdate.MaxUnavailable = &maxSurge + } + + // Cannot allow maxSurge==0 && MaxUnavailable==0, otherwise, no pod can be updated when rolling update. + maxSurge, _ := intstr.GetScaledValueFromIntOrPercent(strategy.RollingUpdate.MaxSurge, 100, true) + maxUnavailable, _ := intstr.GetScaledValueFromIntOrPercent(strategy.RollingUpdate.MaxUnavailable, 100, true) + if maxSurge == 0 && maxUnavailable == 0 { + strategy.RollingUpdate = &apps.RollingUpdateDeployment{ + MaxSurge: &intstr.IntOrString{Type: intstr.Int, IntVal: 0}, + MaxUnavailable: &intstr.IntOrString{Type: intstr.Int, IntVal: 1}, + } + } + +} diff --git a/api/v1beta1/rollout_types.go b/api/v1beta1/rollout_types.go index fcf61682..36e38bf6 100644 --- a/api/v1beta1/rollout_types.go +++ b/api/v1beta1/rollout_types.go @@ -75,6 +75,92 @@ type RolloutStrategy struct { Paused bool `json:"paused,omitempty"` // +optional Canary *CanaryStrategy `json:"canary,omitempty"` + // +optional + BlueGreen *BlueGreenStrategy `json:"blueGreen,omitempty" protobuf:"bytes,1,opt,name=blueGreen"` +} + +// Get the rolling style based on the strategy +func (r *RolloutStrategy) GetRollingStyle() RollingStyleType { + if r.BlueGreen != nil { + return BlueGreenRollingStyle + } + //NOTE - even EnableExtraWorkloadForCanary is true, as long as it is not Deployment, + //we won't do canary release. BatchRelease will treat it as Partiton release + if r.Canary.EnableExtraWorkloadForCanary { + return CanaryRollingStyle + } + return PartitionRollingStyle +} + +// r.GetRollingStyle() == BlueGreenRollingStyle +func (r *RolloutStrategy) IsBlueGreenRelease() bool { + return r.GetRollingStyle() == BlueGreenRollingStyle +} + +// r.GetRollingStyle() == CanaryRollingStyle || r.GetRollingStyle() == PartitionRollingStyle +func (r *RolloutStrategy) IsCanaryStragegy() bool { + return r.GetRollingStyle() == CanaryRollingStyle || r.GetRollingStyle() == PartitionRollingStyle +} + +// Get the steps based on the rolling style +func (r *RolloutStrategy) GetSteps() []CanaryStep { + switch r.GetRollingStyle() { + case BlueGreenRollingStyle: + return r.BlueGreen.Steps + case CanaryRollingStyle, PartitionRollingStyle: + return r.Canary.Steps + default: + return nil + } +} + +// Get the traffic routing based on the rolling style +func (r *RolloutStrategy) GetTrafficRouting() []TrafficRoutingRef { + switch r.GetRollingStyle() { + case BlueGreenRollingStyle: + return r.BlueGreen.TrafficRoutings + case CanaryRollingStyle, PartitionRollingStyle: + return r.Canary.TrafficRoutings + default: + return nil + } +} + +// Check if there are traffic routings +func (r *RolloutStrategy) HasTrafficRoutings() bool { + return len(r.GetTrafficRouting()) > 0 +} + +// Check the value of DisableGenerateCanaryService +func (r *RolloutStrategy) DisableGenerateCanaryService() bool { + switch r.GetRollingStyle() { + case BlueGreenRollingStyle: + return r.BlueGreen.DisableGenerateCanaryService + case CanaryRollingStyle, PartitionRollingStyle: + return r.Canary.DisableGenerateCanaryService + default: + return false + } +} + +// BlueGreenStrategy defines parameters for Blue Green Release +type BlueGreenStrategy struct { + // Steps define the order of phases to execute release in batches(20%, 40%, 60%, 80%, 100%) + // +optional + Steps []CanaryStep `json:"steps,omitempty"` + // TrafficRoutings support ingress, gateway api and custom network resource(e.g. istio, apisix) to enable more fine-grained traffic routing + // and current only support one TrafficRouting + TrafficRoutings []TrafficRoutingRef `json:"trafficRoutings,omitempty"` + // FailureThreshold indicates how many failed pods can be tolerated in all upgraded pods. + // Only when FailureThreshold are satisfied, Rollout can enter ready state. + // If FailureThreshold is nil, Rollout will use the MaxUnavailable of workload as its + // FailureThreshold. + // Defaults to nil. + FailureThreshold *intstr.IntOrString `json:"failureThreshold,omitempty"` + // TrafficRoutingRef is TrafficRouting's Name + TrafficRoutingRef string `json:"trafficRoutingRef,omitempty"` + // canary service will not be generated if DisableGenerateCanaryService is true + DisableGenerateCanaryService bool `json:"disableGenerateCanaryService,omitempty"` } // CanaryStrategy defines parameters for a Replica Based Canary @@ -206,6 +292,9 @@ type RolloutStatus struct { // Canary describes the state of the canary rollout // +optional CanaryStatus *CanaryStatus `json:"canaryStatus,omitempty"` + // BlueGreen describes the state of the blueGreen rollout + // +optional + BlueGreenStatus *BlueGreenStatus `json:"blueGreenStatus,omitempty"` // Conditions a list of conditions a rollout can have. // +optional Conditions []RolloutCondition `json:"conditions,omitempty"` @@ -259,10 +348,26 @@ const ( // Terminating Reason TerminatingReasonInTerminating = "InTerminating" TerminatingReasonCompleted = "Completed" + + // Finalise Reason + // Finalise when the last batch is released and all pods will update to new version + FinaliseReasonSuccess = "Success" + // Finalise when rollback detected + FinaliseReasonRollback = "Rollback" + // Finalise when Continuous Release detected + FinaliseReasonContinuous = "Continuous" + // Finalise when Rollout is disabling + FinaliseReasonDisalbed = "RolloutDisabled" + // Finalise when Rollout is deleting + FinaliseReasonDelete = "RolloutDeleting" ) -// CanaryStatus status fields that only pertain to the canary rollout -type CanaryStatus struct { +// fields in CommonStatus are shared between canary status and bluegreen status +// if a field is accessed in strategy-agnostic way, e.g. accessed from rollout_progressing.go, or rollout_status.go +// then it can be put into CommonStatus +// if a field is only accessed in strategy-specific way, e.g. accessed from rollout_canary.go or rollout_bluegreen.go +// then it should stay behind with CanaryStatus or BlueGreenStatus +type CommonStatus struct { // observedWorkloadGeneration is the most recent generation observed for this Rollout ref workload generation. ObservedWorkloadGeneration int64 `json:"observedWorkloadGeneration,omitempty"` // ObservedRolloutID will record the newest spec.RolloutID if status.canaryRevision equals to workload.updateRevision @@ -271,27 +376,126 @@ type CanaryStatus struct { RolloutHash string `json:"rolloutHash,omitempty"` // StableRevision indicates the revision of stable pods StableRevision string `json:"stableRevision,omitempty"` + // pod template hash is used as service selector label + PodTemplateHash string `json:"podTemplateHash"` + // CurrentStepIndex defines the current step of the rollout is on. + // +optional + CurrentStepIndex int32 `json:"currentStepIndex"` + // NextStepIndex defines the next step of the rollout is on. + // In normal case, NextStepIndex is equal to CurrentStepIndex + 1 + // If the current step is the last step, NextStepIndex is equal to -1 + // Before the release, NextStepIndex is also equal to -1 + // 0 is not used and won't appear in any case + // It is allowed to patch NextStepIndex by design, + // e.g. if CurrentStepIndex is 2, user can patch NextStepIndex to 3 (if exists) to + // achieve batch jump, or patch NextStepIndex to 1 to implement a re-execution of step 1 + // Patching it with a non-positive value is useless and meaningless, which will be corrected + // in the next reconciliation + NextStepIndex int32 `json:"nextStepIndex"` + // FinalisingStep the step of finalising + FinalisingStep FinalisingStepType `json:"finalisingStep"` + CurrentStepState CanaryStepState `json:"currentStepState"` + Message string `json:"message,omitempty"` + LastUpdateTime *metav1.Time `json:"lastUpdateTime,omitempty"` +} + +// CanaryStatus status fields that only pertain to the canary rollout +type CanaryStatus struct { + // must be inline + CommonStatus `json:",inline"` // CanaryRevision is calculated by rollout based on podTemplateHash, and the internal logic flow uses // It may be different from rs podTemplateHash in different k8s versions, so it cannot be used as service selector label CanaryRevision string `json:"canaryRevision"` - // pod template hash is used as service selector label - PodTemplateHash string `json:"podTemplateHash"` // CanaryReplicas the numbers of canary revision pods CanaryReplicas int32 `json:"canaryReplicas"` // CanaryReadyReplicas the numbers of ready canary revision pods CanaryReadyReplicas int32 `json:"canaryReadyReplicas"` - // CurrentStepIndex defines the current step of the rollout is on. If the current step index is null, the - // controller will execute the rollout. - // +optional - CurrentStepIndex int32 `json:"currentStepIndex"` - CurrentStepState CanaryStepState `json:"currentStepState"` - Message string `json:"message,omitempty"` - LastUpdateTime *metav1.Time `json:"lastUpdateTime,omitempty"` +} + +// BlueGreenStatus status fields that only pertain to the blueGreen rollout +type BlueGreenStatus struct { + CommonStatus `json:",inline"` + // CanaryRevision is calculated by rollout based on podTemplateHash, and the internal logic flow uses + // It may be different from rs podTemplateHash in different k8s versions, so it cannot be used as service selector label + UpdatedRevision string `json:"updatedRevision"` + // UpdatedReplicas the numbers of updated pods + UpdatedReplicas int32 `json:"updatedReplicas"` + // UpdatedReadyReplicas the numbers of updated ready pods + UpdatedReadyReplicas int32 `json:"updatedReadyReplicas"` +} + +// GetSubStatus returns the ethier canary or bluegreen status +func (r *RolloutStatus) GetSubStatus() *CommonStatus { + if r.CanaryStatus != nil { + return &(r.CanaryStatus.CommonStatus) + } + return &(r.BlueGreenStatus.CommonStatus) +} + +func (r *RolloutStatus) IsSubStatusEmpty() bool { + return r.CanaryStatus == nil && r.BlueGreenStatus == nil +} + +func (r *RolloutStatus) Clear() { + r.CanaryStatus = nil + r.BlueGreenStatus = nil +} + +//TODO - the following functions seem awkward, is there better way for our case? + +func (r *RolloutStatus) GetCanaryRevision() string { + if r.CanaryStatus != nil { + return r.CanaryStatus.CanaryRevision + } + return r.BlueGreenStatus.UpdatedRevision +} + +func (r *RolloutStatus) SetCanaryRevision(revision string) { + if r.CanaryStatus != nil { + r.CanaryStatus.CanaryRevision = revision + } + if r.BlueGreenStatus != nil { + r.BlueGreenStatus.UpdatedRevision = revision + } +} + +func (r *RolloutStatus) GetCanaryReplicas() int32 { + if r.CanaryStatus != nil { + return r.CanaryStatus.CanaryReplicas + } + return r.BlueGreenStatus.UpdatedReplicas +} + +func (r *RolloutStatus) SetCanaryReplicas(replicas int32) { + if r.CanaryStatus != nil { + r.CanaryStatus.CanaryReplicas = replicas + } + if r.BlueGreenStatus != nil { + r.BlueGreenStatus.UpdatedReplicas = replicas + } +} + +func (r *RolloutStatus) GetCanaryReadyReplicas() int32 { + if r.CanaryStatus != nil { + return r.CanaryStatus.CanaryReadyReplicas + } + return r.BlueGreenStatus.UpdatedReadyReplicas +} + +func (r *RolloutStatus) SetCanaryReadyReplicas(replicas int32) { + if r.CanaryStatus != nil { + r.CanaryStatus.CanaryReadyReplicas = replicas + } + if r.BlueGreenStatus != nil { + r.BlueGreenStatus.UpdatedReadyReplicas = replicas + } } type CanaryStepState string const ( + // the first step, handle some special cases before step upgrade, to prevent traffic loss + CanaryStepStateInit CanaryStepState = "BeforeStepUpgrade" CanaryStepStateUpgrade CanaryStepState = "StepUpgrade" CanaryStepStateTrafficRouting CanaryStepState = "StepTrafficRouting" CanaryStepStateMetricsAnalysis CanaryStepState = "StepMetricsAnalysis" @@ -318,6 +522,34 @@ const ( RolloutPhaseDisabling RolloutPhase = "Disabling" ) +type FinalisingStepType string + +const ( + // some work that should be done before pod scaling down. + // For BlueGreenStrategy: + // we rout all traffic to stable or new version based on FinaliseReason + // For CanaryStrategy: + // we remove the selector of stable service + FinalisingStepTypePreparing FinalisingStepType = "Preparing" + // Patch Batch Release to scale down (exception: the canary Deployment will be + // scaled down in FinalisingStepTypeDeleteBR step) + // For Both BlueGreenStrategy and CanaryStrategy: + // set workload.pause=false, set workload.partition=0 + FinalisingStepTypeBatchRelease FinalisingStepType = "PatchBatchRelease" + //TODO - Currently, the next three steps are in the same function, FinalisingTrafficRouting + // we should try to separate the FinalisingStepTypeGateway and FinalisingStepTypeCanaryService + // with graceful time to prevent some potential issues + + // Restore the stable Service (i.e. remove corresponding selector) + FinalisingStepTypeStableService FinalisingStepType = "RestoreStableService" + // Restore the GatewayAPI/Ingress/Istio + FinalisingStepTypeGateway FinalisingStepType = "RestoreGateway" + // Delete Canary Service + FinalisingStepTypeCanaryService FinalisingStepType = "DeleteCanayService" + // Delete Batch Release + FinalisingStepTypeDeleteBR FinalisingStepType = "DeleteBatchRelease" +) + // +genclient //+kubebuilder:object:root=true //+kubebuilder:subresource:status diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go index 2201cc70..06c081ef 100644 --- a/api/v1beta1/zz_generated.deepcopy.go +++ b/api/v1beta1/zz_generated.deepcopy.go @@ -157,12 +157,59 @@ func (in *BatchReleaseStatus) DeepCopy() *BatchReleaseStatus { } // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *CanaryStatus) DeepCopyInto(out *CanaryStatus) { +func (in *BlueGreenStatus) DeepCopyInto(out *BlueGreenStatus) { *out = *in - if in.LastUpdateTime != nil { - in, out := &in.LastUpdateTime, &out.LastUpdateTime - *out = (*in).DeepCopy() + in.CommonStatus.DeepCopyInto(&out.CommonStatus) +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new BlueGreenStatus. +func (in *BlueGreenStatus) DeepCopy() *BlueGreenStatus { + if in == nil { + return nil + } + out := new(BlueGreenStatus) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *BlueGreenStrategy) DeepCopyInto(out *BlueGreenStrategy) { + *out = *in + if in.Steps != nil { + in, out := &in.Steps, &out.Steps + *out = make([]CanaryStep, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } + if in.TrafficRoutings != nil { + in, out := &in.TrafficRoutings, &out.TrafficRoutings + *out = make([]TrafficRoutingRef, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } + if in.FailureThreshold != nil { + in, out := &in.FailureThreshold, &out.FailureThreshold + *out = new(intstr.IntOrString) + **out = **in + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new BlueGreenStrategy. +func (in *BlueGreenStrategy) DeepCopy() *BlueGreenStrategy { + if in == nil { + return nil } + out := new(BlueGreenStrategy) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *CanaryStatus) DeepCopyInto(out *CanaryStatus) { + *out = *in + in.CommonStatus.DeepCopyInto(&out.CommonStatus) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new CanaryStatus. @@ -236,6 +283,25 @@ func (in *CanaryStrategy) DeepCopy() *CanaryStrategy { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *CommonStatus) DeepCopyInto(out *CommonStatus) { + *out = *in + if in.LastUpdateTime != nil { + in, out := &in.LastUpdateTime, &out.LastUpdateTime + *out = (*in).DeepCopy() + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new CommonStatus. +func (in *CommonStatus) DeepCopy() *CommonStatus { + if in == nil { + return nil + } + out := new(CommonStatus) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *DeploymentExtraStatus) DeepCopyInto(out *DeploymentExtraStatus) { *out = *in @@ -356,6 +422,31 @@ func (in *ObjectRef) DeepCopy() *ObjectRef { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *OriginalDeploymentStrategy) DeepCopyInto(out *OriginalDeploymentStrategy) { + *out = *in + if in.Strategy != nil { + in, out := &in.Strategy, &out.Strategy + *out = new(v1.DeploymentStrategy) + (*in).DeepCopyInto(*out) + } + if in.ProgressDeadlineSeconds != nil { + in, out := &in.ProgressDeadlineSeconds, &out.ProgressDeadlineSeconds + *out = new(int32) + **out = **in + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new OriginalDeploymentStrategy. +func (in *OriginalDeploymentStrategy) DeepCopy() *OriginalDeploymentStrategy { + if in == nil { + return nil + } + out := new(OriginalDeploymentStrategy) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *PatchPodTemplateMetadata) DeepCopyInto(out *PatchPodTemplateMetadata) { *out = *in @@ -557,6 +648,11 @@ func (in *RolloutStatus) DeepCopyInto(out *RolloutStatus) { *out = new(CanaryStatus) (*in).DeepCopyInto(*out) } + if in.BlueGreenStatus != nil { + in, out := &in.BlueGreenStatus, &out.BlueGreenStatus + *out = new(BlueGreenStatus) + (*in).DeepCopyInto(*out) + } if in.Conditions != nil { in, out := &in.Conditions, &out.Conditions *out = make([]RolloutCondition, len(*in)) @@ -584,6 +680,11 @@ func (in *RolloutStrategy) DeepCopyInto(out *RolloutStrategy) { *out = new(CanaryStrategy) (*in).DeepCopyInto(*out) } + if in.BlueGreen != nil { + in, out := &in.BlueGreen, &out.BlueGreen + *out = new(BlueGreenStrategy) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new RolloutStrategy. diff --git a/config/crd/bases/rollouts.kruise.io_batchreleases.yaml b/config/crd/bases/rollouts.kruise.io_batchreleases.yaml index 7eb29e17..25ca8aa8 100644 --- a/config/crd/bases/rollouts.kruise.io_batchreleases.yaml +++ b/config/crd/bases/rollouts.kruise.io_batchreleases.yaml @@ -88,6 +88,14 @@ spec: - canaryReplicas type: object type: array + enableExtraWorkloadForCanary: + description: EnableExtraWorkloadForCanary indicates whether to + create extra workload for canary True corresponds to RollingStyle + "Canary". False corresponds to RollingStyle "Partiton". Ignored + in BlueGreen-style. This field is about to deprecate, use RollingStyle + instead. If both of them are set, controller will only consider + this filed when RollingStyle is empty + type: boolean failureThreshold: anyOf: - type: integer @@ -118,9 +126,14 @@ spec: description: labels type: object type: object + rollingStyle: + description: RollingStyle can be "Canary", "Partiton" or "BlueGreen" + type: string rolloutID: description: RolloutID indicates an id for each rollout progress type: string + required: + - enableExtraWorkloadForCanary type: object targetReference: description: TargetRef contains the GVK and name of the workload that @@ -346,11 +359,12 @@ spec: type: object type: array enableExtraWorkloadForCanary: - description: 'If true, then it will create new deployment for - canary, such as: workload-demo-canary. When user verifies that - the canary version is ready, we will remove the canary deployment - and release the deployment workload-demo in full. Current only - support k8s native deployment' + description: EnableExtraWorkloadForCanary indicates whether to + create extra workload for canary True corresponds to RollingStyle + "Canary". False corresponds to RollingStyle "Partiton". Ignored + in BlueGreen-style. This field is about to deprecate, use RollingStyle + instead. If both of them are set, controller will only consider + this filed when RollingStyle is empty type: boolean failureThreshold: anyOf: @@ -382,6 +396,9 @@ spec: description: labels type: object type: object + rollingStyle: + description: RollingStyle can be "Canary", "Partiton" or "BlueGreen" + type: string rolloutID: description: RolloutID indicates an id for each rollout progress type: string diff --git a/config/crd/bases/rollouts.kruise.io_rollouts.yaml b/config/crd/bases/rollouts.kruise.io_rollouts.yaml index ad1f51a8..aa01c4c1 100644 --- a/config/crd/bases/rollouts.kruise.io_rollouts.yaml +++ b/config/crd/bases/rollouts.kruise.io_rollouts.yaml @@ -435,18 +435,32 @@ spec: so it cannot be used as service selector label type: string currentStepIndex: - description: CurrentStepIndex defines the current step of the - rollout is on. If the current step index is null, the controller - will execute the rollout. format: int32 type: integer currentStepState: type: string + finalisingStep: + type: string lastUpdateTime: format: date-time type: string message: type: string + nextStepIndex: + description: NextStepIndex defines the next step of the rollout + is on. In normal case, NextStepIndex is equal to CurrentStepIndex + + 1 If the current step is the last step, NextStepIndex is equal + to -1 Before the release, NextStepIndex is also equal to -1 + 0 is not used and won't appear in any case It is allowed to + patch NextStepIndex by design, e.g. if CurrentStepIndex is 2, + user can patch NextStepIndex to 3 (if exists) to achieve batch + jump, or patch NextStepIndex to 1 to implement a re-execution + of step 1 Patching it with a non-positive value is meaningless, + which will be corrected in the next reconciliation achieve batch + jump, or patch NextStepIndex to 1 to implement a re-execution + of step 1 + format: int32 + type: integer observedRolloutID: description: ObservedRolloutID will record the newest spec.RolloutID if status.canaryRevision equals to workload.updateRevision @@ -470,6 +484,8 @@ spec: - canaryReplicas - canaryRevision - currentStepState + - finalisingStep + - nextStepIndex - podTemplateHash type: object conditions: @@ -574,6 +590,308 @@ spec: strategy: description: rollout strategy properties: + blueGreen: + description: BlueGreenStrategy defines parameters for Blue Green + Release + properties: + disableGenerateCanaryService: + description: canary service will not be generated if DisableGenerateCanaryService + is true + type: boolean + failureThreshold: + anyOf: + - type: integer + - type: string + description: FailureThreshold indicates how many failed pods + can be tolerated in all upgraded pods. Only when FailureThreshold + are satisfied, Rollout can enter ready state. If FailureThreshold + is nil, Rollout will use the MaxUnavailable of workload + as its FailureThreshold. Defaults to nil. + x-kubernetes-int-or-string: true + steps: + description: Steps define the order of phases to execute release + in batches(20%, 40%, 60%, 80%, 100%) + items: + description: CanaryStep defines a step of a canary workload. + properties: + matches: + description: Matches define conditions used for matching + the incoming HTTP requests to canary service. Each + match is independent, i.e. this rule will be matched + if **any** one of the matches is satisfied. If Gateway + API, current only support one match. And cannot support + both weight and matches, if both are configured, then + matches takes precedence. + items: + properties: + headers: + description: Headers specifies HTTP request header + matchers. Multiple match values are ANDed together, + meaning, a request must match all the specified + headers to select the route. + items: + description: HTTPHeaderMatch describes how to + select a HTTP route by matching HTTP request + headers. + properties: + name: + description: "Name is the name of the HTTP + Header to be matched. Name matching MUST + be case insensitive. (See https://tools.ietf.org/html/rfc7230#section-3.2). + \n If multiple entries specify equivalent + header names, only the first entry with + an equivalent name MUST be considered + for a match. Subsequent entries with an + equivalent header name MUST be ignored. + Due to the case-insensitivity of header + names, \"foo\" and \"Foo\" are considered + equivalent. \n When a header is repeated + in an HTTP request, it is implementation-specific + behavior as to how this is represented. + Generally, proxies should follow the guidance + from the RFC: https://www.rfc-editor.org/rfc/rfc7230.html#section-3.2.2 + regarding processing a repeated header, + with special handling for \"Set-Cookie\"." + maxLength: 256 + minLength: 1 + pattern: ^[A-Za-z0-9!#$%&'*+\-.^_\x60|~]+$ + type: string + type: + default: Exact + description: "Type specifies how to match + against the value of the header. \n Support: + Core (Exact) \n Support: Custom (RegularExpression) + \n Since RegularExpression HeaderMatchType + has custom conformance, implementations + can support POSIX, PCRE or any other dialects + of regular expressions. Please read the + implementation's documentation to determine + the supported dialect." + enum: + - Exact + - RegularExpression + type: string + value: + description: Value is the value of HTTP + Header to be matched. + maxLength: 4096 + minLength: 1 + type: string + required: + - name + - value + type: object + maxItems: 16 + type: array + type: object + type: array + pause: + description: Pause defines a pause stage for a rollout, + manual or auto + properties: + duration: + description: Duration the amount of time to wait + before moving to the next step. + format: int32 + type: integer + type: object + replicas: + anyOf: + - type: integer + - type: string + description: 'Replicas is the number of expected canary + pods in this batch it can be an absolute number (ex: + 5) or a percentage of total pods.' + x-kubernetes-int-or-string: true + requestHeaderModifier: + description: "Set overwrites the request with the given + header (name, value) before the action. \n Input: + \ GET /foo HTTP/1.1 my-header: foo \n requestHeaderModifier: + \ set: - name: \"my-header\" value: \"bar\" + \n Output: GET /foo HTTP/1.1 my-header: bar" + properties: + add: + description: "Add adds the given header(s) (name, + value) to the request before the action. It appends + to any existing values associated with the header + name. \n Input: GET /foo HTTP/1.1 my-header: + foo \n Config: add: - name: \"my-header\" + \ value: \"bar\" \n Output: GET /foo HTTP/1.1 + \ my-header: foo my-header: bar" + items: + description: HTTPHeader represents an HTTP Header + name and value as defined by RFC 7230. + properties: + name: + description: "Name is the name of the HTTP + Header to be matched. Name matching MUST + be case insensitive. (See https://tools.ietf.org/html/rfc7230#section-3.2). + \n If multiple entries specify equivalent + header names, the first entry with an equivalent + name MUST be considered for a match. Subsequent + entries with an equivalent header name MUST + be ignored. Due to the case-insensitivity + of header names, \"foo\" and \"Foo\" are + considered equivalent." + maxLength: 256 + minLength: 1 + pattern: ^[A-Za-z0-9!#$%&'*+\-.^_\x60|~]+$ + type: string + value: + description: Value is the value of HTTP Header + to be matched. + maxLength: 4096 + minLength: 1 + type: string + required: + - name + - value + type: object + maxItems: 16 + type: array + x-kubernetes-list-map-keys: + - name + x-kubernetes-list-type: map + remove: + description: "Remove the given header(s) from the + HTTP request before the action. The value of Remove + is a list of HTTP header names. Note that the + header names are case-insensitive (see https://datatracker.ietf.org/doc/html/rfc2616#section-4.2). + \n Input: GET /foo HTTP/1.1 my-header1: foo + \ my-header2: bar my-header3: baz \n Config: + \ remove: [\"my-header1\", \"my-header3\"] \n + Output: GET /foo HTTP/1.1 my-header2: bar" + items: + type: string + maxItems: 16 + type: array + set: + description: "Set overwrites the request with the + given header (name, value) before the action. + \n Input: GET /foo HTTP/1.1 my-header: foo + \n Config: set: - name: \"my-header\" value: + \"bar\" \n Output: GET /foo HTTP/1.1 my-header: + bar" + items: + description: HTTPHeader represents an HTTP Header + name and value as defined by RFC 7230. + properties: + name: + description: "Name is the name of the HTTP + Header to be matched. Name matching MUST + be case insensitive. (See https://tools.ietf.org/html/rfc7230#section-3.2). + \n If multiple entries specify equivalent + header names, the first entry with an equivalent + name MUST be considered for a match. Subsequent + entries with an equivalent header name MUST + be ignored. Due to the case-insensitivity + of header names, \"foo\" and \"Foo\" are + considered equivalent." + maxLength: 256 + minLength: 1 + pattern: ^[A-Za-z0-9!#$%&'*+\-.^_\x60|~]+$ + type: string + value: + description: Value is the value of HTTP Header + to be matched. + maxLength: 4096 + minLength: 1 + type: string + required: + - name + - value + type: object + maxItems: 16 + type: array + x-kubernetes-list-map-keys: + - name + x-kubernetes-list-type: map + type: object + traffic: + description: Traffic indicate how many percentage of + traffic the canary pods should receive Value is of + string type and is a percentage, e.g. 5%. + type: string + type: object + type: array + trafficRoutingRef: + description: TrafficRoutingRef is TrafficRouting's Name + type: string + trafficRoutings: + description: TrafficRoutings support ingress, gateway api + and custom network resource(e.g. istio, apisix) to enable + more fine-grained traffic routing and current only support + one TrafficRouting + items: + description: TrafficRoutingRef hosts all the different configuration + for supported service meshes to enable more fine-grained + traffic routing + properties: + customNetworkRefs: + description: CustomNetworkRefs hold a list of custom + providers to route traffic + items: + description: ObjectRef holds a references to the Kubernetes + object + properties: + apiVersion: + description: API Version of the referent + type: string + kind: + description: Kind of the referent + type: string + name: + description: Name of the referent + type: string + required: + - apiVersion + - kind + - name + type: object + type: array + gateway: + description: Gateway holds Gateway specific configuration + to route traffic Gateway configuration only supports + >= v0.4.0 (v1alpha2). + properties: + httpRouteName: + description: HTTPRouteName refers to the name of + an `HTTPRoute` resource in the same namespace + as the `Rollout` + type: string + type: object + gracePeriodSeconds: + description: Optional duration in seconds the traffic + provider(e.g. nginx ingress controller) consumes the + service, ingress configuration changes gracefully. + format: int32 + type: integer + ingress: + description: Ingress holds Ingress specific configuration + to route traffic, e.g. Nginx, Alb. + properties: + classType: + description: ClassType refers to the type of `Ingress`. + current support nginx, aliyun-alb. default is + nginx. + type: string + name: + description: Name refers to the name of an `Ingress` + resource in the same namespace as the `Rollout` + type: string + required: + - name + type: object + service: + description: Service holds the name of a service which + selects pods with stable version and don't select + any pods with canary version. + type: string + required: + - service + type: object + type: array + type: object canary: description: CanaryStrategy defines parameters for a Replica Based Canary @@ -1013,6 +1331,79 @@ spec: status: description: RolloutStatus defines the observed state of Rollout properties: + blueGreenStatus: + description: BlueGreen describes the state of the blueGreen rollout + properties: + currentStepIndex: + description: CurrentStepIndex defines the current step of the + rollout is on. + format: int32 + type: integer + currentStepState: + type: string + finalisingStep: + description: FinalisingStep the step of finalising + type: string + lastUpdateTime: + format: date-time + type: string + message: + type: string + nextStepIndex: + description: NextStepIndex defines the next step of the rollout + is on. In normal case, NextStepIndex is equal to CurrentStepIndex + + 1 If the current step is the last step, NextStepIndex is equal + to -1 Before the release, NextStepIndex is also equal to -1 + 0 is not used and won't appear in any case It is allowed to + patch NextStepIndex by design, e.g. if CurrentStepIndex is 2, + user can patch NextStepIndex to 3 (if exists) to achieve batch + jump, or patch NextStepIndex to 1 to implement a re-execution + of step 1 Patching it with a non-positive value is useless and + meaningless, which will be corrected in the next reconciliation + format: int32 + type: integer + observedRolloutID: + description: ObservedRolloutID will record the newest spec.RolloutID + if status.canaryRevision equals to workload.updateRevision + type: string + observedWorkloadGeneration: + description: observedWorkloadGeneration is the most recent generation + observed for this Rollout ref workload generation. + format: int64 + type: integer + podTemplateHash: + description: pod template hash is used as service selector label + type: string + rolloutHash: + description: RolloutHash from rollout.spec object + type: string + stableRevision: + description: StableRevision indicates the revision of stable pods + type: string + updatedReadyReplicas: + description: UpdatedReadyReplicas the numbers of updated ready + pods + format: int32 + type: integer + updatedReplicas: + description: UpdatedReplicas the numbers of updated pods + format: int32 + type: integer + updatedRevision: + description: CanaryRevision is calculated by rollout based on + podTemplateHash, and the internal logic flow uses It may be + different from rs podTemplateHash in different k8s versions, + so it cannot be used as service selector label + type: string + required: + - currentStepState + - finalisingStep + - nextStepIndex + - podTemplateHash + - updatedReadyReplicas + - updatedReplicas + - updatedRevision + type: object canaryStatus: description: Canary describes the state of the canary rollout properties: @@ -1033,17 +1424,32 @@ spec: type: string currentStepIndex: description: CurrentStepIndex defines the current step of the - rollout is on. If the current step index is null, the controller - will execute the rollout. + rollout is on. format: int32 type: integer currentStepState: type: string + finalisingStep: + description: FinalisingStep the step of finalising + type: string lastUpdateTime: format: date-time type: string message: type: string + nextStepIndex: + description: NextStepIndex defines the next step of the rollout + is on. In normal case, NextStepIndex is equal to CurrentStepIndex + + 1 If the current step is the last step, NextStepIndex is equal + to -1 Before the release, NextStepIndex is also equal to -1 + 0 is not used and won't appear in any case It is allowed to + patch NextStepIndex by design, e.g. if CurrentStepIndex is 2, + user can patch NextStepIndex to 3 (if exists) to achieve batch + jump, or patch NextStepIndex to 1 to implement a re-execution + of step 1 Patching it with a non-positive value is useless and + meaningless, which will be corrected in the next reconciliation + format: int32 + type: integer observedRolloutID: description: ObservedRolloutID will record the newest spec.RolloutID if status.canaryRevision equals to workload.updateRevision @@ -1067,6 +1473,8 @@ spec: - canaryReplicas - canaryRevision - currentStepState + - finalisingStep + - nextStepIndex - podTemplateHash type: object conditions: diff --git a/pkg/controller/batchrelease/batchrelease_controller_test.go b/pkg/controller/batchrelease/batchrelease_controller_test.go index c339b74b..8f79493e 100644 --- a/pkg/controller/batchrelease/batchrelease_controller_test.go +++ b/pkg/controller/batchrelease/batchrelease_controller_test.go @@ -67,8 +67,8 @@ var ( Name: "sample", }, ReleasePlan: v1beta1.ReleasePlan{ - EnableExtraWorkloadForCanary: true, - BatchPartition: pointer.Int32(0), + RollingStyle: v1beta1.CanaryRollingStyle, + BatchPartition: pointer.Int32(0), Batches: []v1beta1.ReleaseBatch{ { CanaryReplicas: intstr.FromString("10%"), @@ -147,6 +147,7 @@ var ( }, ReleasePlan: v1beta1.ReleasePlan{ BatchPartition: pointer.Int32Ptr(0), + RollingStyle: v1beta1.PartitionRollingStyle, Batches: []v1beta1.ReleaseBatch{ { CanaryReplicas: intstr.FromString("10%"), diff --git a/pkg/controller/batchrelease/batchrelease_executor.go b/pkg/controller/batchrelease/batchrelease_executor.go index e1aa563e..5082817d 100644 --- a/pkg/controller/batchrelease/batchrelease_executor.go +++ b/pkg/controller/batchrelease/batchrelease_executor.go @@ -197,28 +197,43 @@ func (r *Executor) getReleaseController(release *v1beta1.BatchRelease, newStatus Namespace: release.Namespace, Name: targetRef.Name, } + rollingStyle := release.Spec.ReleasePlan.RollingStyle + if len(rollingStyle) == 0 && release.Spec.ReleasePlan.EnableExtraWorkloadForCanary { + rollingStyle = v1beta1.CanaryRollingStyle + } + klog.Infof("BatchRelease(%v) using %s-style release controller for this batch release", klog.KObj(release), rollingStyle) + switch rollingStyle { + case v1beta1.BlueGreenRollingStyle: + // if targetRef.APIVersion == appsv1alpha1.GroupVersion.String() && targetRef.Kind == reflect.TypeOf(appsv1alpha1.CloneSet{}).Name() { + // klog.InfoS("Using CloneSet bluegreen-style release controller for this batch release", "workload name", targetKey.Name, "namespace", targetKey.Namespace) + // return partitionstyle.NewControlPlane(cloneset.NewController, r.client, r.recorder, release, newStatus, targetKey, gvk), nil + // } + // if targetRef.APIVersion == apps.SchemeGroupVersion.String() && targetRef.Kind == reflect.TypeOf(apps.Deployment{}).Name() { + // klog.InfoS("Using Deployment bluegreen-style release controller for this batch release", "workload name", targetKey.Name, "namespace", targetKey.Namespace) + // return bluegreenstyle.NewControlPlane(deployment.NewController, r.client, r.recorder, release, newStatus, targetKey, gvk), nil + // } + + case v1beta1.CanaryRollingStyle: + if targetRef.APIVersion == apps.SchemeGroupVersion.String() && targetRef.Kind == reflect.TypeOf(apps.Deployment{}).Name() { + klog.InfoS("Using Deployment canary-style release controller for this batch release", "workload name", targetKey.Name, "namespace", targetKey.Namespace) + return canarystyle.NewControlPlane(canarydeployment.NewController, r.client, r.recorder, release, newStatus, targetKey), nil + } + fallthrough - switch targetRef.APIVersion { - case appsv1alpha1.GroupVersion.String(): - if targetRef.Kind == reflect.TypeOf(appsv1alpha1.CloneSet{}).Name() { + case v1beta1.PartitionRollingStyle, "": + if targetRef.APIVersion == appsv1alpha1.GroupVersion.String() && targetRef.Kind == reflect.TypeOf(appsv1alpha1.CloneSet{}).Name() { klog.InfoS("Using CloneSet partition-style release controller for this batch release", "workload name", targetKey.Name, "namespace", targetKey.Namespace) return partitionstyle.NewControlPlane(cloneset.NewController, r.client, r.recorder, release, newStatus, targetKey, gvk), nil } - if targetRef.Kind == reflect.TypeOf(appsv1alpha1.DaemonSet{}).Name() { + if targetRef.APIVersion == appsv1alpha1.GroupVersion.String() && targetRef.Kind == reflect.TypeOf(appsv1alpha1.DaemonSet{}).Name() { klog.InfoS("Using DaemonSet partition-style release controller for this batch release", "workload name", targetKey.Name, "namespace", targetKey.Namespace) return partitionstyle.NewControlPlane(daemonset.NewController, r.client, r.recorder, release, newStatus, targetKey, gvk), nil } - - case apps.SchemeGroupVersion.String(): - if targetRef.Kind == reflect.TypeOf(apps.Deployment{}).Name() { - if !release.Spec.ReleasePlan.EnableExtraWorkloadForCanary { - klog.InfoS("Using Deployment partition-style release controller for this batch release", "workload name", targetKey.Name, "namespace", targetKey.Namespace) - return partitionstyle.NewControlPlane(partitiondeployment.NewController, r.client, r.recorder, release, newStatus, targetKey, gvk), nil - } else { - klog.InfoS("Using Deployment canary-style release controller for this batch release", "workload name", targetKey.Name, "namespace", targetKey.Namespace) - return canarystyle.NewControlPlane(canarydeployment.NewController, r.client, r.recorder, release, newStatus, targetKey), nil - } + if targetRef.APIVersion == apps.SchemeGroupVersion.String() && targetRef.Kind == reflect.TypeOf(apps.Deployment{}).Name() { + klog.InfoS("Using Deployment partition-style release controller for this batch release", "workload name", targetKey.Name, "namespace", targetKey.Namespace) + return partitionstyle.NewControlPlane(partitiondeployment.NewController, r.client, r.recorder, release, newStatus, targetKey, gvk), nil } + klog.Info("Partition, but use StatefulSet-Like partition-style release controller for this batch release") } // try to use StatefulSet-like rollout controller by default diff --git a/pkg/controller/rollout/rollout_canary.go b/pkg/controller/rollout/rollout_canary.go index 41ed5f90..6f623610 100644 --- a/pkg/controller/rollout/rollout_canary.go +++ b/pkg/controller/rollout/rollout_canary.go @@ -364,6 +364,7 @@ func createBatchRelease(rollout *v1beta1.Rollout, rolloutID string, batch int32, BatchPartition: utilpointer.Int32Ptr(batch), FailureThreshold: rollout.Spec.Strategy.Canary.FailureThreshold, PatchPodTemplateMetadata: rollout.Spec.Strategy.Canary.PatchPodTemplateMetadata, + RollingStyle: rollout.Spec.Strategy.GetRollingStyle(), EnableExtraWorkloadForCanary: rollout.Spec.Strategy.Canary.EnableExtraWorkloadForCanary, }, }, diff --git a/pkg/controller/rollout/rollout_canary_test.go b/pkg/controller/rollout/rollout_canary_test.go index 89148109..aebcfeef 100644 --- a/pkg/controller/rollout/rollout_canary_test.go +++ b/pkg/controller/rollout/rollout_canary_test.go @@ -100,6 +100,7 @@ func TestRunCanary(t *testing.T) { } br.Spec.ReleasePlan.BatchPartition = utilpointer.Int32(0) br.Spec.ReleasePlan.EnableExtraWorkloadForCanary = true + br.Spec.ReleasePlan.RollingStyle = v1beta1.CanaryRollingStyle return br }, }, @@ -159,9 +160,12 @@ func TestRunCanary(t *testing.T) { } br.Spec.ReleasePlan.BatchPartition = utilpointer.Int32(0) br.Spec.ReleasePlan.EnableExtraWorkloadForCanary = true + br.Spec.ReleasePlan.RollingStyle = v1beta1.CanaryRollingStyle br.Status = v1beta1.BatchReleaseStatus{ - ObservedGeneration: 1, - ObservedReleasePlanHash: "d444a1007776da957d7d8549e3375c96179621b85670ad1e2bb0fc5fea16446a", + ObservedGeneration: 1, + // since we use RollingStyle over EnableExtraWorkloadForCanary now, former hardcoded hash + // should be re-calculated + ObservedReleasePlanHash: util.HashReleasePlanBatches(&br.Spec.ReleasePlan), CanaryStatus: v1beta1.BatchReleaseCanaryStatus{ CurrentBatchState: v1beta1.ReadyBatchState, CurrentBatch: 0, @@ -205,6 +209,7 @@ func TestRunCanary(t *testing.T) { } br.Spec.ReleasePlan.BatchPartition = utilpointer.Int32(0) br.Spec.ReleasePlan.EnableExtraWorkloadForCanary = true + br.Spec.ReleasePlan.RollingStyle = v1beta1.CanaryRollingStyle return br }, }, diff --git a/pkg/controller/rollout/rollout_progressing.go b/pkg/controller/rollout/rollout_progressing.go index 0aaf5975..2388eb46 100644 --- a/pkg/controller/rollout/rollout_progressing.go +++ b/pkg/controller/rollout/rollout_progressing.go @@ -66,17 +66,31 @@ func (r *RolloutReconciler) reconcileRolloutProgressing(rollout *v1beta1.Rollout switch cond.Reason { case v1alpha1.ProgressingReasonInitializing: klog.Infof("rollout(%s/%s) is Progressing, and in reason(%s)", rollout.Namespace, rollout.Name, cond.Reason) - // new canaryStatus - newStatus.CanaryStatus = &v1beta1.CanaryStatus{ + // clear and create + newStatus.Clear() + commonStatus := v1beta1.CommonStatus{ ObservedWorkloadGeneration: rolloutContext.Workload.Generation, RolloutHash: rolloutContext.Rollout.Annotations[util.RolloutHashAnnotation], ObservedRolloutID: getRolloutID(rolloutContext.Workload), StableRevision: rolloutContext.Workload.StableRevision, - CanaryRevision: rolloutContext.Workload.CanaryRevision, CurrentStepIndex: 1, - CurrentStepState: v1beta1.CanaryStepStateUpgrade, + NextStepIndex: util.NextBatchIndex(rollout, 1), + CurrentStepState: v1beta1.CanaryStepStateInit, LastUpdateTime: &metav1.Time{Time: time.Now()}, } + if rollout.Spec.Strategy.IsBlueGreenRelease() { + newStatus.BlueGreenStatus = &v1beta1.BlueGreenStatus{ + CommonStatus: commonStatus, + UpdatedRevision: rolloutContext.Workload.CanaryRevision, + } + } else { + commonStatus.CurrentStepState = v1beta1.CanaryStepStateUpgrade + newStatus.CanaryStatus = &v1beta1.CanaryStatus{ + CommonStatus: commonStatus, + CanaryRevision: rolloutContext.Workload.CanaryRevision, + } + } + done, err := r.doProgressingInitializing(rolloutContext) if err != nil { klog.Errorf("rollout(%s/%s) doProgressingInitializing error(%s)", rollout.Namespace, rollout.Name, err.Error()) diff --git a/pkg/controller/rollout/rollout_progressing_test.go b/pkg/controller/rollout/rollout_progressing_test.go index e9649394..42412b3e 100644 --- a/pkg/controller/rollout/rollout_progressing_test.go +++ b/pkg/controller/rollout/rollout_progressing_test.go @@ -67,6 +67,7 @@ func TestReconcileRolloutProgressing(t *testing.T) { s.CanaryStatus.StableRevision = "pod-template-hash-v1" s.CanaryStatus.CanaryRevision = "6f8cc56547" s.CanaryStatus.CurrentStepIndex = 1 + s.CanaryStatus.NextStepIndex = 2 s.CanaryStatus.CurrentStepState = v1beta1.CanaryStepStateUpgrade return s }, @@ -99,6 +100,7 @@ func TestReconcileRolloutProgressing(t *testing.T) { s.CanaryStatus.StableRevision = "pod-template-hash-v1" s.CanaryStatus.CanaryRevision = "6f8cc56547" s.CanaryStatus.CurrentStepIndex = 1 + s.CanaryStatus.NextStepIndex = 2 s.CanaryStatus.CurrentStepState = v1beta1.CanaryStepStateUpgrade cond := util.GetRolloutCondition(*s, v1beta1.RolloutConditionProgressing) cond.Reason = v1alpha1.ProgressingReasonInRolling diff --git a/pkg/controller/rollout/rollout_status.go b/pkg/controller/rollout/rollout_status.go index 8165bcc4..0e33522e 100755 --- a/pkg/controller/rollout/rollout_status.go +++ b/pkg/controller/rollout/rollout_status.go @@ -123,17 +123,28 @@ func (r *RolloutReconciler) calculateRolloutStatus(rollout *v1beta1.Rollout) (re // But at the first deployment of Rollout/Workload, CanaryStatus isn't set due to no rollout progression, // and PaaS platform cannot judge whether the deployment is completed base on the code above. So we have // to update the status just like the rollout was completed. - - newStatus.CanaryStatus = &v1beta1.CanaryStatus{ + commonStatus := v1beta1.CommonStatus{ ObservedRolloutID: getRolloutID(workload), ObservedWorkloadGeneration: workload.Generation, PodTemplateHash: workload.PodTemplateHash, - CanaryRevision: workload.CanaryRevision, StableRevision: workload.StableRevision, - CurrentStepIndex: int32(len(rollout.Spec.Strategy.Canary.Steps)), + CurrentStepIndex: int32(len(rollout.Spec.Strategy.GetSteps())), + NextStepIndex: util.NextBatchIndex(rollout, int32(len(rollout.Spec.Strategy.GetSteps()))), CurrentStepState: v1beta1.CanaryStepStateCompleted, RolloutHash: rollout.Annotations[util.RolloutHashAnnotation], } + if rollout.Spec.Strategy.IsBlueGreenRelease() { + newStatus.BlueGreenStatus = &v1beta1.BlueGreenStatus{ + CommonStatus: commonStatus, + UpdatedRevision: workload.CanaryRevision, + } + } else { + newStatus.CanaryStatus = &v1beta1.CanaryStatus{ + CommonStatus: commonStatus, + CanaryRevision: workload.CanaryRevision, + } + } + newStatus.Message = "workload deployment is completed" } case v1beta1.RolloutPhaseDisabled: diff --git a/pkg/trafficrouting/manager_test.go b/pkg/trafficrouting/manager_test.go index 366585e1..8ff7ce88 100644 --- a/pkg/trafficrouting/manager_test.go +++ b/pkg/trafficrouting/manager_test.go @@ -163,15 +163,17 @@ var ( Status: v1beta1.RolloutStatus{ Phase: v1beta1.RolloutPhaseProgressing, CanaryStatus: &v1beta1.CanaryStatus{ - ObservedWorkloadGeneration: 1, - RolloutHash: "rollout-hash-v1", - ObservedRolloutID: "rollout-id-1", - StableRevision: "podtemplatehash-v1", - CanaryRevision: "revision-v2", - CurrentStepIndex: 1, - CurrentStepState: v1beta1.CanaryStepStateTrafficRouting, - PodTemplateHash: "podtemplatehash-v2", - LastUpdateTime: &metav1.Time{Time: time.Now()}, + CommonStatus: v1beta1.CommonStatus{ + ObservedWorkloadGeneration: 1, + RolloutHash: "rollout-hash-v1", + ObservedRolloutID: "rollout-id-1", + StableRevision: "podtemplatehash-v1", + CurrentStepIndex: 1, + CurrentStepState: v1beta1.CanaryStepStateTrafficRouting, + PodTemplateHash: "podtemplatehash-v2", + LastUpdateTime: &metav1.Time{Time: time.Now()}, + }, + CanaryRevision: "revision-v2", }, Conditions: []v1beta1.RolloutCondition{ { @@ -249,15 +251,17 @@ var ( Status: v1beta1.RolloutStatus{ Phase: v1beta1.RolloutPhaseProgressing, CanaryStatus: &v1beta1.CanaryStatus{ - ObservedWorkloadGeneration: 1, - RolloutHash: "rollout-hash-v1", - ObservedRolloutID: "rollout-id-1", - StableRevision: "podtemplatehash-v1", - CanaryRevision: "revision-v2", - CurrentStepIndex: 1, - CurrentStepState: v1beta1.CanaryStepStateTrafficRouting, - PodTemplateHash: "podtemplatehash-v2", - LastUpdateTime: &metav1.Time{Time: time.Now()}, + CommonStatus: v1beta1.CommonStatus{ + ObservedWorkloadGeneration: 1, + RolloutHash: "rollout-hash-v1", + ObservedRolloutID: "rollout-id-1", + StableRevision: "podtemplatehash-v1", + CurrentStepIndex: 1, + CurrentStepState: v1beta1.CanaryStepStateTrafficRouting, + PodTemplateHash: "podtemplatehash-v2", + LastUpdateTime: &metav1.Time{Time: time.Now()}, + }, + CanaryRevision: "revision-v2", }, Conditions: []v1beta1.RolloutCondition{ { diff --git a/pkg/util/rollout_utils.go b/pkg/util/rollout_utils.go index f41d7bf7..927be3a1 100644 --- a/pkg/util/rollout_utils.go +++ b/pkg/util/rollout_utils.go @@ -164,3 +164,15 @@ func DumpJSON(o interface{}) string { func EncodeHash(data string) string { return fmt.Sprintf("%x", sha256.Sum256([]byte(data))) } + +// calculate the next batch index +func NextBatchIndex(rollout *rolloutv1beta1.Rollout, CurrentStepIndex int32) int32 { + if rollout == nil { + return -1 + } + allSteps := int32(len(rollout.Spec.Strategy.GetSteps())) + if CurrentStepIndex >= allSteps { + return -1 + } + return CurrentStepIndex + 1 +} diff --git a/pkg/webhook/rollout/validating/rollout_create_update_handler.go b/pkg/webhook/rollout/validating/rollout_create_update_handler.go index 0d2c78b9..43db8927 100644 --- a/pkg/webhook/rollout/validating/rollout_create_update_handler.go +++ b/pkg/webhook/rollout/validating/rollout_create_update_handler.go @@ -133,12 +133,11 @@ func (h *RolloutCreateUpdateHandler) validateRolloutUpdate(oldObj, newObj *appsv if !reflect.DeepEqual(oldObj.Spec.WorkloadRef, newObj.Spec.WorkloadRef) { return field.ErrorList{field.Forbidden(field.NewPath("Spec.ObjectRef"), "Rollout 'ObjectRef' field is immutable")} } - // canary strategy - if !reflect.DeepEqual(oldObj.Spec.Strategy.Canary.TrafficRoutings, newObj.Spec.Strategy.Canary.TrafficRoutings) { - return field.ErrorList{field.Forbidden(field.NewPath("Spec.Strategy.Canary.TrafficRoutings"), "Rollout 'Strategy.Canary.TrafficRoutings' field is immutable")} + if !reflect.DeepEqual(oldObj.Spec.Strategy.GetTrafficRouting(), newObj.Spec.Strategy.GetTrafficRouting()) { + return field.ErrorList{field.Forbidden(field.NewPath("Spec.Strategy.Canary|BlueGreen.TrafficRoutings"), "Rollout 'Strategy.Canary|BlueGreen.TrafficRoutings' field is immutable")} } - if oldObj.Spec.Strategy.Canary.EnableExtraWorkloadForCanary != newObj.Spec.Strategy.Canary.EnableExtraWorkloadForCanary { - return field.ErrorList{field.Forbidden(field.NewPath("Spec.Strategy.Canary"), "Rollout enableExtraWorkloadForCanary is immutable")} + if oldObj.Spec.Strategy.GetRollingStyle() != newObj.Spec.Strategy.GetRollingStyle() { + return field.ErrorList{field.Forbidden(field.NewPath("Spec.Strategy.Canary|BlueGreen"), "Rollout style and enableExtraWorkloadForCanary are immutable")} } } @@ -198,15 +197,32 @@ func validateRolloutSpecObjectRef(workloadRef *appsv1beta1.ObjectRef, fldPath *f } func validateRolloutSpecStrategy(strategy *appsv1beta1.RolloutStrategy, fldPath *field.Path) field.ErrorList { + if strategy.Canary == nil && strategy.BlueGreen == nil { + return field.ErrorList{field.Invalid(fldPath, nil, "Canary and BlueGreen cannot both be empty")} + } + if strategy.Canary != nil && strategy.BlueGreen != nil { + return field.ErrorList{field.Invalid(fldPath, nil, "Canary and BlueGreen cannot both be set")} + } + if strategy.BlueGreen != nil { + return validateRolloutSpecBlueGreenStrategy(strategy.BlueGreen, fldPath.Child("BlueGreen")) + } return validateRolloutSpecCanaryStrategy(strategy.Canary, fldPath.Child("Canary")) } +type TrafficRule string + +const ( + TrafficRuleCanary TrafficRule = "Canary" + TrafficRuleBlueGreen TrafficRule = "BlueGreen" + NoTraffic TrafficRule = "NoTraffic" +) + func validateRolloutSpecCanaryStrategy(canary *appsv1beta1.CanaryStrategy, fldPath *field.Path) field.ErrorList { - if canary == nil { - return field.ErrorList{field.Invalid(fldPath, nil, "Canary cannot be empty")} + trafficRule := NoTraffic + if len(canary.TrafficRoutings) > 0 { + trafficRule = TrafficRuleCanary } - - errList := validateRolloutSpecCanarySteps(canary.Steps, fldPath.Child("Steps"), len(canary.TrafficRoutings) > 0) + errList := validateRolloutSpecCanarySteps(canary.Steps, fldPath.Child("Steps"), trafficRule) if len(canary.TrafficRoutings) > 1 { errList = append(errList, field.Invalid(fldPath, canary.TrafficRoutings, "Rollout currently only support single TrafficRouting.")) } @@ -216,6 +232,21 @@ func validateRolloutSpecCanaryStrategy(canary *appsv1beta1.CanaryStrategy, fldPa return errList } +func validateRolloutSpecBlueGreenStrategy(blueGreen *appsv1beta1.BlueGreenStrategy, fldPath *field.Path) field.ErrorList { + trafficRule := NoTraffic + if len(blueGreen.TrafficRoutings) > 0 { + trafficRule = TrafficRuleBlueGreen + } + errList := validateRolloutSpecCanarySteps(blueGreen.Steps, fldPath.Child("Steps"), trafficRule) + if len(blueGreen.TrafficRoutings) > 1 { + errList = append(errList, field.Invalid(fldPath, blueGreen.TrafficRoutings, "Rollout currently only support single TrafficRouting.")) + } + for _, traffic := range blueGreen.TrafficRoutings { + errList = append(errList, validateRolloutSpecCanaryTraffic(traffic, fldPath.Child("TrafficRouting"))...) + } + return errList +} + func validateRolloutSpecCanaryTraffic(traffic appsv1beta1.TrafficRoutingRef, fldPath *field.Path) field.ErrorList { errList := field.ErrorList{} if len(traffic.Service) == 0 { @@ -240,7 +271,7 @@ func validateRolloutSpecCanaryTraffic(traffic appsv1beta1.TrafficRoutingRef, fld return errList } -func validateRolloutSpecCanarySteps(steps []appsv1beta1.CanaryStep, fldPath *field.Path, isTraffic bool) field.ErrorList { +func validateRolloutSpecCanarySteps(steps []appsv1beta1.CanaryStep, fldPath *field.Path, trafficRule TrafficRule) field.ErrorList { stepCount := len(steps) if stepCount == 0 { return field.ErrorList{field.Invalid(fldPath, steps, "The number of Canary.Steps cannot be empty")} @@ -258,14 +289,21 @@ func validateRolloutSpecCanarySteps(steps []appsv1beta1.CanaryStep, fldPath *fie return field.ErrorList{field.Invalid(fldPath.Index(i).Child("Replicas"), s.Replicas, `replicas must be positive number, or a percentage with "0%" < canaryReplicas <= "100%"`)} } - if !isTraffic { + if trafficRule == NoTraffic || s.Traffic == nil { continue } - if s.Traffic != nil { - is := intstr.FromString(*s.Traffic) - weight, err := intstr.GetScaledValueFromIntOrPercent(&is, 100, true) + is := intstr.FromString(*s.Traffic) + weight, err := intstr.GetScaledValueFromIntOrPercent(&is, 100, true) + switch trafficRule { + case TrafficRuleBlueGreen: + // traffic "0%" is allowed in blueGreen strategy + if err != nil || weight < 0 || weight > 100 { + return field.ErrorList{field.Invalid(fldPath.Index(i).Child("steps"), steps, `traffic must be percentage with "0%" <= traffic <= "100%" in blueGreen strategy`)} + } + default: + // traffic "0%" is not allowed in canary strategy if err != nil || weight <= 0 || weight > 100 { - return field.ErrorList{field.Invalid(fldPath.Index(i).Child("steps"), steps, `traffic must be percentage with "0%" < traffic <= "100%"`)} + return field.ErrorList{field.Invalid(fldPath.Index(i).Child("steps"), steps, `traffic must be percentage with "0%" < traffic <= "100%" in canary strategy`)} } } } diff --git a/pkg/webhook/rollout/validating/rollout_create_update_handler_test.go b/pkg/webhook/rollout/validating/rollout_create_update_handler_test.go index 07eacd57..27ea4531 100644 --- a/pkg/webhook/rollout/validating/rollout_create_update_handler_test.go +++ b/pkg/webhook/rollout/validating/rollout_create_update_handler_test.go @@ -96,7 +96,9 @@ var ( }, Status: appsv1beta1.RolloutStatus{ CanaryStatus: &appsv1beta1.CanaryStatus{ - CurrentStepState: appsv1beta1.CanaryStepStateCompleted, + CommonStatus: appsv1beta1.CommonStatus{ + CurrentStepState: appsv1beta1.CanaryStepStateCompleted, + }, }, }, } diff --git a/test/e2e/deployment_test.go b/test/e2e/deployment_test.go index b857f929..c8379bc4 100644 --- a/test/e2e/deployment_test.go +++ b/test/e2e/deployment_test.go @@ -26,6 +26,16 @@ import ( var _ = SIGDescribe("Advanced Deployment", func() { var namespace string + + DumpAllResources := func() { + deploy := &apps.DeploymentList{} + k8sClient.List(context.TODO(), deploy, client.InNamespace(namespace)) + fmt.Println(util.DumpJSON(deploy)) + rs := &apps.ReplicaSetList{} + k8sClient.List(context.TODO(), rs, client.InNamespace(namespace)) + fmt.Println(util.DumpJSON(rs)) + } + defaultRetry := wait.Backoff{ Steps: 10, Duration: 10 * time.Millisecond, @@ -132,7 +142,12 @@ var _ = SIGDescribe("Advanced Deployment", func() { CheckReplicas := func(deployment *apps.Deployment, replicas, available, updated int32) { var clone *apps.Deployment + start := time.Now() Eventually(func() bool { + if start.Add(time.Minute * 2).Before(time.Now()) { + DumpAllResources() + Expect(true).Should(BeFalse()) + } clone = &apps.Deployment{} err := GetObject(deployment.Namespace, deployment.Name, clone) Expect(err).NotTo(HaveOccurred()) @@ -239,6 +254,7 @@ var _ = SIGDescribe("Advanced Deployment", func() { deployment.Namespace = namespace Expect(ReadYamlToObject("./test_data/deployment/deployment.yaml", deployment)).ToNot(HaveOccurred()) CreateObject(deployment) + CheckReplicas(deployment, 5, 5, 5) UpdateDeployment(deployment, "version2") UpdatePartitionWithCheck(deployment, intstr.FromInt(0)) UpdatePartitionWithCheck(deployment, intstr.FromInt(1)) @@ -255,6 +271,7 @@ var _ = SIGDescribe("Advanced Deployment", func() { Expect(ReadYamlToObject("./test_data/deployment/deployment.yaml", deployment)).ToNot(HaveOccurred()) deployment.Spec.Replicas = pointer.Int32(10) CreateObject(deployment) + CheckReplicas(deployment, 10, 10, 10) UpdateDeployment(deployment, "version2") UpdatePartitionWithCheck(deployment, intstr.FromString("0%")) UpdatePartitionWithCheck(deployment, intstr.FromString("40%")) @@ -287,6 +304,7 @@ var _ = SIGDescribe("Advanced Deployment", func() { `{"rollingStyle":"Partition","rollingUpdate":{"maxUnavailable":1,"maxSurge":0}}` deployment.Spec.MinReadySeconds = 10 CreateObject(deployment) + CheckReplicas(deployment, 5, 5, 5) UpdateDeployment(deployment, "version2") UpdatePartitionWithCheck(deployment, intstr.FromInt(0)) UpdatePartitionWithoutCheck(deployment, intstr.FromInt(3)) @@ -303,6 +321,7 @@ var _ = SIGDescribe("Advanced Deployment", func() { deployment.Namespace = namespace Expect(ReadYamlToObject("./test_data/deployment/deployment.yaml", deployment)).ToNot(HaveOccurred()) CreateObject(deployment) + CheckReplicas(deployment, 5, 5, 5) UpdateDeployment(deployment, "version2") UpdatePartitionWithCheck(deployment, intstr.FromInt(0)) UpdatePartitionWithCheck(deployment, intstr.FromInt(2)) @@ -317,6 +336,7 @@ var _ = SIGDescribe("Advanced Deployment", func() { deployment.Namespace = namespace Expect(ReadYamlToObject("./test_data/deployment/deployment.yaml", deployment)).ToNot(HaveOccurred()) CreateObject(deployment) + CheckReplicas(deployment, 5, 5, 5) UpdateDeployment(deployment, "version2") UpdatePartitionWithCheck(deployment, intstr.FromInt(0)) UpdatePartitionWithCheck(deployment, intstr.FromInt(2)) @@ -335,6 +355,7 @@ var _ = SIGDescribe("Advanced Deployment", func() { Expect(ReadYamlToObject("./test_data/deployment/deployment.yaml", deployment)).ToNot(HaveOccurred()) deployment.Annotations["rollouts.kruise.io/deployment-strategy"] = `{"rollingUpdate":{"maxUnavailable":0,"maxSurge":1}}` CreateObject(deployment) + CheckReplicas(deployment, 5, 5, 5) UpdateDeployment(deployment, "version2", "busybox:not-exists") UpdatePartitionWithoutCheck(deployment, intstr.FromInt(1)) CheckReplicas(deployment, 6, 5, 1) diff --git a/test/e2e/rollout_test.go b/test/e2e/rollout_test.go index caea11bb..c9ad8b34 100644 --- a/test/e2e/rollout_test.go +++ b/test/e2e/rollout_test.go @@ -215,18 +215,25 @@ var _ = SIGDescribe("Rollout", func() { } ResumeRolloutCanary := func(name string) { + clone := &v1alpha1.Rollout{} + Expect(GetObject(name, clone)).NotTo(HaveOccurred()) + currentIndex := clone.Status.CanaryStatus.CurrentStepIndex Eventually(func() bool { clone := &v1alpha1.Rollout{} Expect(GetObject(name, clone)).NotTo(HaveOccurred()) - if clone.Status.CanaryStatus.CurrentStepState != v1alpha1.CanaryStepStatePaused { + if clone.Status.CanaryStatus.CurrentStepIndex == currentIndex && clone.Status.CanaryStatus.CurrentStepState == v1alpha1.CanaryStepStatePaused { + klog.Info("patch to stepReady") + body := fmt.Sprintf(`{"status":{"canaryStatus":{"currentStepState":"%s"}}}`, v1alpha1.CanaryStepStateReady) + Expect(k8sClient.Status().Patch(context.TODO(), clone, client.RawPatch(types.MergePatchType, []byte(body)))).NotTo(HaveOccurred()) + return false + } else { fmt.Println("resume rollout success, and CurrentStepState", util.DumpJSON(clone.Status)) return true } - - body := fmt.Sprintf(`{"status":{"canaryStatus":{"currentStepState":"%s"}}}`, v1alpha1.CanaryStepStateReady) - Expect(k8sClient.Status().Patch(context.TODO(), clone, client.RawPatch(types.MergePatchType, []byte(body)))).NotTo(HaveOccurred()) - return false - }, 10*time.Second, time.Second).Should(BeTrue()) + // interval was critical before: + // too small: StepReady could be overidden by StepPaused + // too big: StepReady could progress to StepPaused of next Step + }, 10*time.Second, 2*time.Second).Should(BeTrue()) } ResumeRolloutCanaryV1beta1 := func(name string) {