From 7a18bfaac59d8eab7aeb21548bebcd9f9ec14923 Mon Sep 17 00:00:00 2001 From: stefanprodan Date: Thu, 4 Jun 2020 12:15:44 +0300 Subject: [PATCH 1/3] Add timeout fields to AppMesh client --- pkg/apis/appmesh/v1beta2/types.go | 24 +++ pkg/apis/appmesh/v1beta2/virtualnode_types.go | 59 +++--- .../appmesh/v1beta2/virtualrouter_types.go | 29 ++- .../appmesh/v1beta2/zz_generated.deepcopy.go | 193 +++++++++++++----- 4 files changed, 205 insertions(+), 100 deletions(-) diff --git a/pkg/apis/appmesh/v1beta2/types.go b/pkg/apis/appmesh/v1beta2/types.go index 324ea351..a94cfa83 100644 --- a/pkg/apis/appmesh/v1beta2/types.go +++ b/pkg/apis/appmesh/v1beta2/types.go @@ -89,3 +89,27 @@ type VirtualGatewayReference struct { // UID is the UID of VirtualGateway CR UID types.UID `json:"uid"` } + +type HTTPTimeout struct { + // An object that represents per request timeout duration. + // +optional + PerRequest *Duration `json:"perRequest,omitempty"` + // An object that represents idle timeout duration. + // +optional + Idle *Duration `json:"idle,omitempty"` +} + +type GRPCTimeout struct { + // An object that represents per request timeout duration. + // +optional + PerRequest *Duration `json:"perRequest,omitempty"` + // An object that represents idle timeout duration. + // +optional + Idle *Duration `json:"idle,omitempty"` +} + +type TCPTimeout struct { + // An object that represents idle timeout duration. + // +optional + Idle *Duration `json:"idle,omitempty"` +} diff --git a/pkg/apis/appmesh/v1beta2/virtualnode_types.go b/pkg/apis/appmesh/v1beta2/virtualnode_types.go index 48b29f48..813806de 100644 --- a/pkg/apis/appmesh/v1beta2/virtualnode_types.go +++ b/pkg/apis/appmesh/v1beta2/virtualnode_types.go @@ -82,41 +82,30 @@ type BackendDefaults struct { // HealthCheckPolicy refers to https://docs.aws.amazon.com/app-mesh/latest/APIReference/API_HealthCheckPolicy.html type HealthCheckPolicy struct { // The number of consecutive successful health checks that must occur before declaring listener healthy. - // If unspecified, defaults to be 10 // +kubebuilder:validation:Minimum=2 // +kubebuilder:validation:Maximum=10 - // +optional - HealthyThreshold *int64 `json:"healthyThreshold,omitempty"` + HealthyThreshold int64 `json:"healthyThreshold"` // The time period in milliseconds between each health check execution. - // If unspecified, defaults to be 30000 // +kubebuilder:validation:Minimum=5000 // +kubebuilder:validation:Maximum=300000 - // +optional - IntervalMillis *int64 `json:"intervalMillis,omitempty"` + IntervalMillis int64 `json:"intervalMillis"` // The destination path for the health check request. // This value is only used if the specified protocol is http or http2. For any other protocol, this value is ignored. // +optional Path *string `json:"path,omitempty"` // The destination port for the health check request. - // If unspecified, defaults to be same as port defined in the PortMapping for the listener. // +optional Port *PortNumber `json:"port,omitempty"` // The protocol for the health check request - // If unspecified, defaults to be same as protocol defined in the PortMapping for the listener. - // +optional - Protocol *PortProtocol `json:"protocol,omitempty"` + Protocol PortProtocol `json:"protocol"` // The amount of time to wait when receiving a response from the health check, in milliseconds. - // If unspecified, defaults to be 5000 // +kubebuilder:validation:Minimum=2000 // +kubebuilder:validation:Maximum=60000 - // +optional - TimeoutMillis *int64 `json:"timeoutMillis,omitempty"` + TimeoutMillis int64 `json:"timeoutMillis"` // The number of consecutive failed health checks that must occur before declaring a virtual node unhealthy. - // If unspecified, defaults to be 2 // +kubebuilder:validation:Minimum=2 // +kubebuilder:validation:Maximum=10 - // +optional - UnhealthyThreshold *int64 `json:"unhealthyThreshold,omitempty"` + UnhealthyThreshold int64 `json:"unhealthyThreshold"` } // ListenerTLSACMCertificate refers to https://docs.aws.amazon.com/app-mesh/latest/APIReference/API_ListenerTlsAcmCertificate.html @@ -164,6 +153,22 @@ type ListenerTLS struct { Mode ListenerTLSMode `json:"mode"` } +// ListenerTimeout refers to https://docs.aws.amazon.com/app-mesh/latest/APIReference/API_ListenerTimeout.html +type ListenerTimeout struct { + // Specifies tcp timeout information for the virtual node. + // +optional + TCP *TCPTimeout `json:"tcp,omitempty"` + // Specifies http timeout information for the virtual node. + // +optional + HTTP *HTTPTimeout `json:"http,omitempty"` + // Specifies http2 information for the virtual node. + // +optional + HTTP2 *HTTPTimeout `json:"http2,omitempty"` + // Specifies grpc timeout information for the virtual node. + // +optional + GRPC *GRPCTimeout `json:"grpc,omitempty"` +} + // Listener refers to https://docs.aws.amazon.com/app-mesh/latest/APIReference/API_Listener.html type Listener struct { // The port mapping information for the listener. @@ -174,6 +179,9 @@ type Listener struct { // A reference to an object that represents the Transport Layer Security (TLS) properties for a listener. // +optional TLS *ListenerTLS `json:"tls,omitempty"` + // A reference to an object that represents + // +optional + Timeout *ListenerTimeout `json:"timeout,omitempty"` } // AWSCloudMapInstanceAttribute refers to https://docs.aws.amazon.com/app-mesh/latest/APIReference/API_AwsCloudMapInstanceAttribute.html @@ -264,16 +272,6 @@ type VirtualNodeCondition struct { Message *string `json:"message,omitempty"` } -// AWSCloudMapServiceStatus is AWS CloudMap Service object's info -type AWSCloudMapServiceStatus struct { - // NamespaceID is AWS CloudMap Service object's namespace Id - // +optional - NamespaceID *string `json:"namespaceID,omitempty"` - // ServiceID is AWS CloudMap Service object's Id - // +optional - ServiceID *string `json:"serviceID,omitempty"` -} - // VirtualNodeSpec defines the desired state of VirtualNode // refers to https://docs.aws.amazon.com/app-mesh/latest/APIReference/API_VirtualServiceSpec.html type VirtualNodeSpec struct { @@ -282,7 +280,9 @@ type VirtualNodeSpec struct { // +optional AWSName *string `json:"awsName,omitempty"` // PodSelector selects Pods using labels to designate VirtualNode membership. - // if unspecified or empty, it selects no pods. + // This field follows standard label selector semantics: + // if present but empty, it selects all pods within namespace. + // if absent, it selects no pod. // +optional PodSelector *metav1.LabelSelector `json:"podSelector,omitempty"` // The listener that the virtual node is expected to receive inbound traffic from @@ -320,9 +320,10 @@ type VirtualNodeStatus struct { // The current VirtualNode status. // +optional Conditions []VirtualNodeCondition `json:"conditions,omitempty"` - // AWSCloudMapServiceStatus is AWS CloudMap Service object's info + + // The generation observed by the VirtualNode controller. // +optional - AWSCloudMapServiceStatus *AWSCloudMapServiceStatus `json:"awsCloudMapServiceStatus,omitempty"` + ObservedGeneration *int64 `json:"observedGeneration,omitempty"` } // +genclient diff --git a/pkg/apis/appmesh/v1beta2/virtualrouter_types.go b/pkg/apis/appmesh/v1beta2/virtualrouter_types.go index e4b3674a..3fe43d53 100644 --- a/pkg/apis/appmesh/v1beta2/virtualrouter_types.go +++ b/pkg/apis/appmesh/v1beta2/virtualrouter_types.go @@ -1,19 +1,3 @@ -/* - - -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 v1beta2 import ( @@ -149,6 +133,9 @@ type HTTPRoute struct { // An object that represents a retry policy. // +optional RetryPolicy *HTTPRetryPolicy `json:"retryPolicy,omitempty"` + // An object that represents a http timeout. + // +optional + Timeout *HTTPTimeout `json:"timeout,omitempty"` } // TCPRouteAction refers to https://docs.aws.amazon.com/app-mesh/latest/APIReference/API_TcpRouteAction.html @@ -163,6 +150,9 @@ type TCPRouteAction struct { type TCPRoute struct { // The action to take if a match is determined. Action TCPRouteAction `json:"action"` + // An object that represents a tcp timeout. + // +optional + Timeout *TCPTimeout `json:"timeout,omitempty"` } // GRPCRouteMetadataMatchMethod refers to https://docs.aws.amazon.com/app-mesh/latest/APIReference/API_GrpcRouteMetadataMatchMethod.html @@ -261,6 +251,9 @@ type GRPCRoute struct { // An object that represents a retry policy. // +optional RetryPolicy *GRPCRetryPolicy `json:"retryPolicy,omitempty"` + // An object that represents a grpc timeout. + // +optional + Timeout *GRPCTimeout `json:"timeout,omitempty"` } // Route refers to https://docs.aws.amazon.com/app-mesh/latest/APIReference/API_RouteSpec.html @@ -345,6 +338,10 @@ type VirtualRouterStatus struct { // The current VirtualRouter status. // +optional Conditions []VirtualRouterCondition `json:"conditions,omitempty"` + + // The generation observed by the VirtualRouter controller. + // +optional + ObservedGeneration *int64 `json:"observedGeneration,omitempty"` } // +genclient diff --git a/pkg/apis/appmesh/v1beta2/zz_generated.deepcopy.go b/pkg/apis/appmesh/v1beta2/zz_generated.deepcopy.go index 9e5fe7dc..0faa568f 100644 --- a/pkg/apis/appmesh/v1beta2/zz_generated.deepcopy.go +++ b/pkg/apis/appmesh/v1beta2/zz_generated.deepcopy.go @@ -62,32 +62,6 @@ func (in *AWSCloudMapServiceDiscovery) DeepCopy() *AWSCloudMapServiceDiscovery { return out } -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *AWSCloudMapServiceStatus) DeepCopyInto(out *AWSCloudMapServiceStatus) { - *out = *in - if in.NamespaceID != nil { - in, out := &in.NamespaceID, &out.NamespaceID - *out = new(string) - **out = **in - } - if in.ServiceID != nil { - in, out := &in.ServiceID, &out.ServiceID - *out = new(string) - **out = **in - } - return -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AWSCloudMapServiceStatus. -func (in *AWSCloudMapServiceStatus) DeepCopy() *AWSCloudMapServiceStatus { - if in == nil { - return nil - } - out := new(AWSCloudMapServiceStatus) - in.DeepCopyInto(out) - return out -} - // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *AccessLog) DeepCopyInto(out *AccessLog) { *out = *in @@ -285,6 +259,11 @@ func (in *GRPCRoute) DeepCopyInto(out *GRPCRoute) { *out = new(GRPCRetryPolicy) (*in).DeepCopyInto(*out) } + if in.Timeout != nil { + in, out := &in.Timeout, &out.Timeout + *out = new(GRPCTimeout) + (*in).DeepCopyInto(*out) + } return } @@ -421,6 +400,32 @@ func (in *GRPCRouteMetadataMatchMethod) DeepCopy() *GRPCRouteMetadataMatchMethod return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *GRPCTimeout) DeepCopyInto(out *GRPCTimeout) { + *out = *in + if in.PerRequest != nil { + in, out := &in.PerRequest, &out.PerRequest + *out = new(Duration) + **out = **in + } + if in.Idle != nil { + in, out := &in.Idle, &out.Idle + *out = new(Duration) + **out = **in + } + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new GRPCTimeout. +func (in *GRPCTimeout) DeepCopy() *GRPCTimeout { + if in == nil { + return nil + } + out := new(GRPCTimeout) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *HTTPRetryPolicy) DeepCopyInto(out *HTTPRetryPolicy) { *out = *in @@ -458,6 +463,11 @@ func (in *HTTPRoute) DeepCopyInto(out *HTTPRoute) { *out = new(HTTPRetryPolicy) (*in).DeepCopyInto(*out) } + if in.Timeout != nil { + in, out := &in.Timeout, &out.Timeout + *out = new(HTTPTimeout) + (*in).DeepCopyInto(*out) + } return } @@ -553,6 +563,32 @@ func (in *HTTPRouteMatch) DeepCopy() *HTTPRouteMatch { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *HTTPTimeout) DeepCopyInto(out *HTTPTimeout) { + *out = *in + if in.PerRequest != nil { + in, out := &in.PerRequest, &out.PerRequest + *out = new(Duration) + **out = **in + } + if in.Idle != nil { + in, out := &in.Idle, &out.Idle + *out = new(Duration) + **out = **in + } + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new HTTPTimeout. +func (in *HTTPTimeout) DeepCopy() *HTTPTimeout { + if in == nil { + return nil + } + out := new(HTTPTimeout) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *HeaderMatchMethod) DeepCopyInto(out *HeaderMatchMethod) { *out = *in @@ -597,16 +633,6 @@ func (in *HeaderMatchMethod) DeepCopy() *HeaderMatchMethod { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *HealthCheckPolicy) DeepCopyInto(out *HealthCheckPolicy) { *out = *in - if in.HealthyThreshold != nil { - in, out := &in.HealthyThreshold, &out.HealthyThreshold - *out = new(int64) - **out = **in - } - if in.IntervalMillis != nil { - in, out := &in.IntervalMillis, &out.IntervalMillis - *out = new(int64) - **out = **in - } if in.Path != nil { in, out := &in.Path, &out.Path *out = new(string) @@ -617,21 +643,6 @@ func (in *HealthCheckPolicy) DeepCopyInto(out *HealthCheckPolicy) { *out = new(PortNumber) **out = **in } - if in.Protocol != nil { - in, out := &in.Protocol, &out.Protocol - *out = new(PortProtocol) - **out = **in - } - if in.TimeoutMillis != nil { - in, out := &in.TimeoutMillis, &out.TimeoutMillis - *out = new(int64) - **out = **in - } - if in.UnhealthyThreshold != nil { - in, out := &in.UnhealthyThreshold, &out.UnhealthyThreshold - *out = new(int64) - **out = **in - } return } @@ -659,6 +670,11 @@ func (in *Listener) DeepCopyInto(out *Listener) { *out = new(ListenerTLS) (*in).DeepCopyInto(*out) } + if in.Timeout != nil { + in, out := &in.Timeout, &out.Timeout + *out = new(ListenerTimeout) + (*in).DeepCopyInto(*out) + } return } @@ -747,6 +763,42 @@ func (in *ListenerTLSFileCertificate) DeepCopy() *ListenerTLSFileCertificate { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ListenerTimeout) DeepCopyInto(out *ListenerTimeout) { + *out = *in + if in.TCP != nil { + in, out := &in.TCP, &out.TCP + *out = new(TCPTimeout) + (*in).DeepCopyInto(*out) + } + if in.HTTP != nil { + in, out := &in.HTTP, &out.HTTP + *out = new(HTTPTimeout) + (*in).DeepCopyInto(*out) + } + if in.HTTP2 != nil { + in, out := &in.HTTP2, &out.HTTP2 + *out = new(HTTPTimeout) + (*in).DeepCopyInto(*out) + } + if in.GRPC != nil { + in, out := &in.GRPC, &out.GRPC + *out = new(GRPCTimeout) + (*in).DeepCopyInto(*out) + } + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ListenerTimeout. +func (in *ListenerTimeout) DeepCopy() *ListenerTimeout { + if in == nil { + return nil + } + out := new(ListenerTimeout) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Logging) DeepCopyInto(out *Logging) { *out = *in @@ -887,6 +939,11 @@ func (in *ServiceDiscovery) DeepCopy() *ServiceDiscovery { func (in *TCPRoute) DeepCopyInto(out *TCPRoute) { *out = *in in.Action.DeepCopyInto(&out.Action) + if in.Timeout != nil { + in, out := &in.Timeout, &out.Timeout + *out = new(TCPTimeout) + (*in).DeepCopyInto(*out) + } return } @@ -923,6 +980,27 @@ func (in *TCPRouteAction) DeepCopy() *TCPRouteAction { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *TCPTimeout) DeepCopyInto(out *TCPTimeout) { + *out = *in + if in.Idle != nil { + in, out := &in.Idle, &out.Idle + *out = new(Duration) + **out = **in + } + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new TCPTimeout. +func (in *TCPTimeout) DeepCopy() *TCPTimeout { + if in == nil { + return nil + } + out := new(TCPTimeout) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *TLSValidationContext) DeepCopyInto(out *TLSValidationContext) { *out = *in @@ -1228,10 +1306,10 @@ func (in *VirtualNodeStatus) DeepCopyInto(out *VirtualNodeStatus) { (*in)[i].DeepCopyInto(&(*out)[i]) } } - if in.AWSCloudMapServiceStatus != nil { - in, out := &in.AWSCloudMapServiceStatus, &out.AWSCloudMapServiceStatus - *out = new(AWSCloudMapServiceStatus) - (*in).DeepCopyInto(*out) + if in.ObservedGeneration != nil { + in, out := &in.ObservedGeneration, &out.ObservedGeneration + *out = new(int64) + **out = **in } return } @@ -1452,6 +1530,11 @@ func (in *VirtualRouterStatus) DeepCopyInto(out *VirtualRouterStatus) { (*in)[i].DeepCopyInto(&(*out)[i]) } } + if in.ObservedGeneration != nil { + in, out := &in.ObservedGeneration, &out.ObservedGeneration + *out = new(int64) + **out = **in + } return } From 37ec07d2ec47373069442b21ca8c7d8205d52ac8 Mon Sep 17 00:00:00 2001 From: stefanprodan Date: Thu, 4 Jun 2020 12:37:34 +0300 Subject: [PATCH 2/3] Set timeout for virtual nodes and routers --- pkg/router/appmesh_v1beta2.go | 45 +++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/pkg/router/appmesh_v1beta2.go b/pkg/router/appmesh_v1beta2.go index 527099fc..c6cfc31f 100644 --- a/pkg/router/appmesh_v1beta2.go +++ b/pkg/router/appmesh_v1beta2.go @@ -78,6 +78,8 @@ func (ar *AppMeshv1beta2Router) Reconcile(canary *flaggerv1.Canary) error { // the virtual node naming format is name-role-namespace func (ar *AppMeshv1beta2Router) reconcileVirtualNode(canary *flaggerv1.Canary, name string, podSelector string, host string) error { protocol := ar.getProtocol(canary) + timeout := ar.makeListenerTimeout(canary) + vnSpec := appmeshv1.VirtualNodeSpec{ Listeners: []appmeshv1.Listener{ { @@ -85,6 +87,7 @@ func (ar *AppMeshv1beta2Router) reconcileVirtualNode(canary *flaggerv1.Canary, n Port: ar.getContainerPort(canary), Protocol: protocol, }, + Timeout: timeout, }, }, ServiceDiscovery: &appmeshv1.ServiceDiscovery{ @@ -170,6 +173,7 @@ func (ar *AppMeshv1beta2Router) reconcileVirtualRouter(canary *flaggerv1.Canary, canaryVirtualNode := fmt.Sprintf("%s-canary", apexName) primaryVirtualNode := fmt.Sprintf("%s-primary", apexName) protocol := ar.getProtocol(canary) + timeout := ar.makeRouteTimeout(canary) routerName := apexName if canaryWeight > 0 { @@ -191,6 +195,7 @@ func (ar *AppMeshv1beta2Router) reconcileVirtualRouter(canary *flaggerv1.Canary, Match: appmeshv1.HTTPRouteMatch{ Prefix: routePrefix, }, + Timeout: timeout, RetryPolicy: ar.makeRetryPolicy(canary), Action: appmeshv1.HTTPRouteAction{ WeightedTargets: []appmeshv1.WeightedTarget{ @@ -223,6 +228,7 @@ func (ar *AppMeshv1beta2Router) reconcileVirtualRouter(canary *flaggerv1.Canary, Prefix: routePrefix, Headers: ar.makeHeaders(canary), }, + Timeout: timeout, RetryPolicy: ar.makeRetryPolicy(canary), Action: appmeshv1.HTTPRouteAction{ WeightedTargets: []appmeshv1.WeightedTarget{ @@ -249,6 +255,7 @@ func (ar *AppMeshv1beta2Router) reconcileVirtualRouter(canary *flaggerv1.Canary, Match: appmeshv1.HTTPRouteMatch{ Prefix: routePrefix, }, + Timeout: timeout, RetryPolicy: ar.makeRetryPolicy(canary), Action: appmeshv1.HTTPRouteAction{ WeightedTargets: []appmeshv1.WeightedTarget{ @@ -445,6 +452,26 @@ func (ar *AppMeshv1beta2Router) SetRoutes( return nil } +// makeRouteTimeout creates an AppMesh HTTPTimeout from the Canary.Service.Timeout +func (ar *AppMeshv1beta2Router) makeRouteTimeout(canary *flaggerv1.Canary) *appmeshv1.HTTPTimeout { + if timeout := ar.getTimeout(canary); timeout != nil { + return &appmeshv1.HTTPTimeout{ + PerRequest: timeout, + } + } + return nil +} + +// makeListenerTimeout creates an AppMesh ListenerTimeout from the Canary.Service.Timeout +func (ar *AppMeshv1beta2Router) makeListenerTimeout(canary *flaggerv1.Canary) *appmeshv1.ListenerTimeout { + if timeout := ar.makeRouteTimeout(canary); timeout != nil { + return &appmeshv1.ListenerTimeout{ + HTTP: timeout, + } + } + return nil +} + // makeRetryPolicy creates an AppMesh HTTPRetryPolicy from the Canary.Service.Retries // default: one retry on gateway error with a 250ms timeout func (ar *AppMeshv1beta2Router) makeRetryPolicy(canary *flaggerv1.Canary) *appmeshv1.HTTPRetryPolicy { @@ -516,6 +543,24 @@ func (ar *AppMeshv1beta2Router) getContainerPort(canary *flaggerv1.Canary) appme return appmeshv1.PortNumber(containerPort) } +// getTimeout converts the Canary.Service.Timeout to AppMesh Duration +func (ar *AppMeshv1beta2Router) getTimeout(canary *flaggerv1.Canary) *appmeshv1.Duration { + if canary.Spec.Service.Timeout != "" { + timeout := int64(1500) + if d, err := time.ParseDuration(canary.Spec.Service.Retries.PerTryTimeout); err == nil { + timeout = d.Milliseconds() + } else { + return nil + } + + return &appmeshv1.Duration{ + Unit: appmeshv1.DurationUnitMS, + Value: timeout, + } + } + return nil +} + func (ar *AppMeshv1beta2Router) gatewayAnnotations(canary *flaggerv1.Canary) map[string]string { a := make(map[string]string) domains := "" From 0715e1ca37a54e07173c69983f35ca4bca00ab8a Mon Sep 17 00:00:00 2001 From: stefanprodan Date: Thu, 4 Jun 2020 13:48:52 +0300 Subject: [PATCH 3/3] Add AppMesh timeout unit test --- pkg/router/appmesh_v1beta2.go | 31 +++++++++++++----------------- pkg/router/appmesh_v1beta2_test.go | 5 +++++ pkg/router/router_test.go | 2 +- 3 files changed, 19 insertions(+), 19 deletions(-) diff --git a/pkg/router/appmesh_v1beta2.go b/pkg/router/appmesh_v1beta2.go index c6cfc31f..cf106eb4 100644 --- a/pkg/router/appmesh_v1beta2.go +++ b/pkg/router/appmesh_v1beta2.go @@ -452,6 +452,19 @@ func (ar *AppMeshv1beta2Router) SetRoutes( return nil } +// getTimeout converts the Canary.Service.Timeout to AppMesh Duration +func (ar *AppMeshv1beta2Router) getTimeout(canary *flaggerv1.Canary) *appmeshv1.Duration { + if canary.Spec.Service.Timeout != "" { + if d, err := time.ParseDuration(canary.Spec.Service.Timeout); err == nil { + return &appmeshv1.Duration{ + Unit: appmeshv1.DurationUnitMS, + Value: d.Milliseconds(), + } + } + } + return nil +} + // makeRouteTimeout creates an AppMesh HTTPTimeout from the Canary.Service.Timeout func (ar *AppMeshv1beta2Router) makeRouteTimeout(canary *flaggerv1.Canary) *appmeshv1.HTTPTimeout { if timeout := ar.getTimeout(canary); timeout != nil { @@ -543,24 +556,6 @@ func (ar *AppMeshv1beta2Router) getContainerPort(canary *flaggerv1.Canary) appme return appmeshv1.PortNumber(containerPort) } -// getTimeout converts the Canary.Service.Timeout to AppMesh Duration -func (ar *AppMeshv1beta2Router) getTimeout(canary *flaggerv1.Canary) *appmeshv1.Duration { - if canary.Spec.Service.Timeout != "" { - timeout := int64(1500) - if d, err := time.ParseDuration(canary.Spec.Service.Retries.PerTryTimeout); err == nil { - timeout = d.Milliseconds() - } else { - return nil - } - - return &appmeshv1.Duration{ - Unit: appmeshv1.DurationUnitMS, - Value: timeout, - } - } - return nil -} - func (ar *AppMeshv1beta2Router) gatewayAnnotations(canary *flaggerv1.Canary) map[string]string { a := make(map[string]string) domains := "" diff --git a/pkg/router/appmesh_v1beta2_test.go b/pkg/router/appmesh_v1beta2_test.go index 9425cc3f..d01b216c 100644 --- a/pkg/router/appmesh_v1beta2_test.go +++ b/pkg/router/appmesh_v1beta2_test.go @@ -52,9 +52,14 @@ func TestAppmeshv1beta2Router_Reconcile(t *testing.T) { vnPrimary, err := router.appmeshClient.AppmeshV1beta2().VirtualNodes("default").Get(context.TODO(), primaryName, metav1.GetOptions{}) require.NoError(t, err) + // check FQDN primaryDNS := fmt.Sprintf("%s.%s.svc.cluster.local.", primaryName, mocks.appmeshCanary.Namespace) assert.Equal(t, primaryDNS, vnPrimary.Spec.ServiceDiscovery.DNS.Hostname) + // check timeout + assert.Equal(t, int64(30000), vrApex.Spec.Routes[0].HTTPRoute.Timeout.PerRequest.Value) + assert.Equal(t, int64(30000), vnPrimary.Spec.Listeners[0].Timeout.HTTP.PerRequest.Value) + // test backends update cd, err := mocks.flaggerClient.FlaggerV1beta1().Canaries("default").Get(context.TODO(), mocks.appmeshCanary.Name, metav1.GetOptions{}) require.NoError(t, err) diff --git a/pkg/router/router_test.go b/pkg/router/router_test.go index 87946ca6..c5635d2f 100644 --- a/pkg/router/router_test.go +++ b/pkg/router/router_test.go @@ -152,7 +152,7 @@ func newTestCanaryAppMesh() *flaggerv1.Canary { MeshName: "global", Hosts: []string{"*"}, Backends: []string{"backend.default"}, - Timeout: "25", + Timeout: "30s", Retries: &istiov1alpha3.HTTPRetry{ Attempts: 5, PerTryTimeout: "gateway-error",