Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support Path and QueryParams in http route matches #204

Merged
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/e2e-custom.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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}')
Expand Down
14 changes: 8 additions & 6 deletions api/v1alpha1/conversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ package v1alpha1
import (
"fmt"

"strings"

"github.com/openkruise/rollouts/api/v1beta1"
"k8s.io/apimachinery/pkg/util/intstr"
utilpointer "k8s.io/utils/pointer"
Expand Down Expand Up @@ -74,7 +76,7 @@ func (src *Rollout) ConvertTo(dst conversion.Hub) error {
obj.Spec.Strategy.Canary.PatchPodTemplateMetadata.Labels[k] = v
}
}
if src.Annotations[RolloutStyleAnnotation] != string(PartitionRollingStyle) {
if !strings.EqualFold(src.Annotations[RolloutStyleAnnotation], string(PartitionRollingStyle)) {
obj.Spec.Strategy.Canary.EnableExtraWorkloadForCanary = true
}
if src.Annotations[TrafficRoutingAnnotation] != "" {
Expand Down Expand Up @@ -211,9 +213,9 @@ func (dst *Rollout) ConvertFrom(src conversion.Hub) error {
dst.Annotations = map[string]string{}
}
if srcV1beta1.Spec.Strategy.Canary.EnableExtraWorkloadForCanary {
dst.Annotations[RolloutStyleAnnotation] = string(CanaryRollingStyle)
dst.Annotations[RolloutStyleAnnotation] = strings.ToLower(string(CanaryRollingStyle))
} else {
dst.Annotations[RolloutStyleAnnotation] = string(PartitionRollingStyle)
dst.Annotations[RolloutStyleAnnotation] = strings.ToLower(string(PartitionRollingStyle))
}
if srcV1beta1.Spec.Strategy.Canary.TrafficRoutingRef != "" {
dst.Annotations[TrafficRoutingAnnotation] = srcV1beta1.Spec.Strategy.Canary.TrafficRoutingRef
Expand Down Expand Up @@ -336,7 +338,7 @@ func (src *BatchRelease) ConvertTo(dst conversion.Hub) error {
obj.Spec.ReleasePlan.PatchPodTemplateMetadata.Labels[k] = v
}
}
if src.Annotations[RolloutStyleAnnotation] != string(PartitionRollingStyle) {
if !strings.EqualFold(src.Annotations[RolloutStyleAnnotation], string(PartitionRollingStyle)) {
obj.Spec.ReleasePlan.EnableExtraWorkloadForCanary = true
}

Expand Down Expand Up @@ -416,9 +418,9 @@ func (dst *BatchRelease) ConvertFrom(src conversion.Hub) error {
dst.Annotations = map[string]string{}
}
if srcV1beta1.Spec.ReleasePlan.EnableExtraWorkloadForCanary {
dst.Annotations[RolloutStyleAnnotation] = string(CanaryRollingStyle)
dst.Annotations[RolloutStyleAnnotation] = strings.ToLower(string(CanaryRollingStyle))
} else {
dst.Annotations[RolloutStyleAnnotation] = string(PartitionRollingStyle)
dst.Annotations[RolloutStyleAnnotation] = strings.ToLower(string(PartitionRollingStyle))
}

// status
Expand Down
2 changes: 2 additions & 0 deletions api/v1alpha1/rollout_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,8 @@ type CanaryStrategy struct {
// only support for canary deployment
// +optional
PatchPodTemplateMetadata *PatchPodTemplateMetadata `json:"patchPodTemplateMetadata,omitempty"`
// canary service will not be generated if DisableGenerateCanaryService is true
DisableGenerateCanaryService bool `json:"disableGenerateCanaryService,omitempty"`
lujiajing1126 marked this conversation as resolved.
Show resolved Hide resolved
}

type PatchPodTemplateMetadata struct {
Expand Down
33 changes: 32 additions & 1 deletion api/v1beta1/rollout_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,8 @@ type CanaryStrategy struct {
EnableExtraWorkloadForCanary bool `json:"enableExtraWorkloadForCanary,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"`
lujiajing1126 marked this conversation as resolved.
Show resolved Hide resolved
}

type PatchPodTemplateMetadata struct {
Expand Down Expand Up @@ -146,17 +148,46 @@ type TrafficRoutingStrategy struct {
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.
//
// For Gateway API, only a single match rule will be applied since Gateway API use ANDed rules if multiple
// ones are defined, i.e. the match will evaluate to true only if all conditions are satisfied.
// Header (for backwards-compatibility) > QueryParams
// And cannot support both weight and matches, if both are configured, then 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
//
// +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
Expand Down
12 changes: 12 additions & 0 deletions api/v1beta1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

101 changes: 96 additions & 5 deletions config/crd/bases/rollouts.kruise.io_rollouts.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,10 @@ spec:
description: CanaryStrategy defines parameters for a Replica Based
Canary
properties:
disableGenerateCanaryService:
description: canary service will not be generated if DisableGenerateCanaryService
is true
type: boolean
failureThreshold:
anyOf:
- type: integer
Expand Down Expand Up @@ -574,6 +578,10 @@ spec:
description: CanaryStrategy defines parameters for a Replica Based
Canary
properties:
disableGenerateCanaryService:
description: canary service will not be generated if DisableGenerateCanaryService
is true
type: boolean
enableExtraWorkloadForCanary:
description: 'If true, then it will create new deployment
for canary, such as: workload-demo-canary. When user verifies
Expand Down Expand Up @@ -614,13 +622,17 @@ spec:
description: CanaryStep defines a step of a canary workload.
properties:
matches:
description: Matches define conditions used for matching
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.
if **any** one of the matches is satisfied. \n For
Gateway API, only a single match rule will be applied
since Gateway API use ANDed rules if multiple ones
are defined, i.e. the match will evaluate to true
only if all conditions are satisfied. Header (for
backwards-compatibility) > QueryParams And cannot
support both weight and matches, if both are configured,
then matches takes precedence."
items:
properties:
headers:
Expand Down Expand Up @@ -682,6 +694,85 @@ 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'
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:
Expand Down
31 changes: 23 additions & 8 deletions lua_configuration/trafficrouting_ingress/mse.lua
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -33,17 +37,28 @@ 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
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
else
annotations["nginx.ingress.kubernetes.io/canary-by-header"] = header.name
if ( header.type == "RegularExpression" )
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
Expand Down
19 changes: 10 additions & 9 deletions pkg/controller/rollout/rollout_progressing.go
Original file line number Diff line number Diff line change
Expand Up @@ -452,14 +452,15 @@ func newTrafficRoutingContext(c *RolloutContext) *trafficrouting.TrafficRoutingC
revisionLabelKey = c.Workload.RevisionLabelKey
}
return &trafficrouting.TrafficRoutingContext{
Key: fmt.Sprintf("Rollout(%s/%s)", c.Rollout.Namespace, c.Rollout.Name),
Namespace: c.Rollout.Namespace,
ObjectRef: c.Rollout.Spec.Strategy.Canary.TrafficRoutings,
Strategy: currentStep.TrafficRoutingStrategy,
OwnerRef: *metav1.NewControllerRef(c.Rollout, rolloutControllerKind),
RevisionLabelKey: revisionLabelKey,
StableRevision: c.NewStatus.CanaryStatus.StableRevision,
CanaryRevision: c.NewStatus.CanaryStatus.PodTemplateHash,
LastUpdateTime: c.NewStatus.CanaryStatus.LastUpdateTime,
Key: fmt.Sprintf("Rollout(%s/%s)", c.Rollout.Namespace, c.Rollout.Name),
Namespace: c.Rollout.Namespace,
ObjectRef: c.Rollout.Spec.Strategy.Canary.TrafficRoutings,
Strategy: currentStep.TrafficRoutingStrategy,
OwnerRef: *metav1.NewControllerRef(c.Rollout, rolloutControllerKind),
RevisionLabelKey: revisionLabelKey,
StableRevision: c.NewStatus.CanaryStatus.StableRevision,
CanaryRevision: c.NewStatus.CanaryStatus.PodTemplateHash,
LastUpdateTime: c.NewStatus.CanaryStatus.LastUpdateTime,
DisableGenerateCanaryService: c.Rollout.Spec.Strategy.Canary.DisableGenerateCanaryService,
}
}
Loading
Loading