Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
4 changes: 4 additions & 0 deletions api/v1alpha2/experimental.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@ package v1alpha2

type Experimental struct {
PilotFeatures `json:"pilot"`

// Enables the Dual Stack support
// +kubebuilder:validation:Optional
EnableDualStack *bool `json:"enableDualStack,omitempty"`
}

type PilotFeatures struct {
Expand Down
62 changes: 61 additions & 1 deletion api/v1alpha2/istio_merge.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package v1alpha2
import (
"encoding/json"
"github.com/golang/protobuf/ptypes/duration"

"istio.io/istio/operator/pkg/values"
"istio.io/istio/pkg/util/protomarshal"
appsv1 "k8s.io/api/apps/v1"
Expand Down Expand Up @@ -77,6 +76,17 @@ func (m *meshConfigBuilder) BuildPrometheusMergeConfig(prometheusMerge bool) *me
return m
}

func (m *meshConfigBuilder) BuildDualStackConfig(enableDualStack bool) *meshConfigBuilder {
if enableDualStack {
err := m.c.SetPath("defaultConfig.proxyMetadata.ISTIO_DUAL_STACK", "true")
if err != nil {
return nil
}
}

return m
}

func (m *meshConfigBuilder) AddProxyMetadata(key, value string) (*meshConfigBuilder, error) {
err := m.c.SetPath("defaultConfig.proxyMetadata."+key, value)
if err != nil {
Expand Down Expand Up @@ -183,15 +193,23 @@ func (i *Istio) mergeConfig(op iopv1alpha1.IstioOperator) (iopv1alpha1.IstioOper
return op, err
}

dualStackEnabled := i.Spec.Experimental != nil && i.Spec.Experimental.EnableDualStack != nil && *i.Spec.Experimental.EnableDualStack

newMeshConfig := mcb.
BuildNumTrustedProxies(i.Spec.Config.NumTrustedProxies).
BuildExternalAuthorizerConfiguration(i.Spec.Config.Authorizers).
BuildPrometheusMergeConfig(i.Spec.Config.Telemetry.Metrics.PrometheusMerge).
BuildDualStackConfig(dualStackEnabled).
Build()

op.Spec.MeshConfig = newMeshConfig

op = applyGatewayExternalTrafficPolicy(op, i)

op, err = applyDualStack(op, i)
if err != nil {
return op, err
}

return op, nil
}
Expand Down Expand Up @@ -228,6 +246,48 @@ func applyGatewayExternalTrafficPolicy(op iopv1alpha1.IstioOperator, i *Istio) i
return op
}

func applyDualStack(op iopv1alpha1.IstioOperator, i *Istio) (iopv1alpha1.IstioOperator, error) {
if i.Spec.Experimental != nil && i.Spec.Experimental.EnableDualStack != nil && *i.Spec.Experimental.EnableDualStack {
return enableDualStack(op, i)
}
return op, nil
}

func enableDualStack(op iopv1alpha1.IstioOperator, i *Istio) (iopv1alpha1.IstioOperator, error) {
valuesMap, err := values.MapFromObject(op.Spec.Values)
if err != nil {
return op, err
}

if valuesMap == nil {
valuesMap = make(values.Map)
}

err = valuesMap.SetPath("pilot.env.ISTIO_DUAL_STACK", "true")
if err != nil {
return iopv1alpha1.IstioOperator{}, err
}
err = valuesMap.SetPath("pilot.ipFamilyPolicy", "RequireDualStack")
if err != nil {
return iopv1alpha1.IstioOperator{}, err
}
err = valuesMap.SetPath("gateways.istio-ingressgateway.ipFamilyPolicy", "RequireDualStack")
if err != nil {
return iopv1alpha1.IstioOperator{}, err
}
err = valuesMap.SetPath("gateways.istio-egressgateway.ipFamilyPolicy", "RequireDualStack")
if err != nil {
return iopv1alpha1.IstioOperator{}, err
}

op.Spec.Values, err = values.ConvertMap[json.RawMessage](valuesMap)
if err != nil {
return op, err
}

return op, nil
}

//nolint:gocognit,gocyclo,cyclop,funlen // cognitive complexity 189 of func `(*Istio).mergeResources` is high (> 20), cyclomatic complexity 70 of func `(*Istio).mergeResources` is high (> 30), Function 'mergeResources' has too many statements (129 > 50) TODO: refactor this function
func (i *Istio) mergeResources(op iopv1alpha1.IstioOperator) (iopv1alpha1.IstioOperator, error) {
if i.Spec.Components == nil {
Expand Down
70 changes: 70 additions & 0 deletions api/v1alpha2/merge_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -664,6 +664,76 @@ var _ = Describe("Merge", func() {

})

It("should set ipFamilyPolicy to RequireDualStack if dualStack is enabled in the Istio CR", func() {
// given
enableDualStack := true
iop := iopv1alpha1.IstioOperator{
Spec: iopv1alpha1.IstioOperatorSpec{},
}
istioCR := istiov1alpha2.Istio{Spec: istiov1alpha2.IstioSpec{
Config: istiov1alpha2.Config{},
Experimental: &istiov1alpha2.Experimental{
EnableDualStack: &enableDualStack,
},
}}

// when
out, err := istioCR.MergeInto(iop)

valuesMap, err := values.MapFromObject(out.Spec.Values)
Expect(err).ShouldNot(HaveOccurred())

Expect(values.TryGetPathAs[string](valuesMap, "pilot.ipFamilyPolicy")).To(Equal("RequireDualStack"))
Expect(values.TryGetPathAs[string](valuesMap, "gateways.istio-ingressgateway.ipFamilyPolicy")).To(Equal("RequireDualStack"))
Expect(values.TryGetPathAs[string](valuesMap, "gateways.istio-egressgateway.ipFamilyPolicy")).To(Equal("RequireDualStack"))
})

It("should set dual stack env for Istio pilot if dualStack is enabled in the Istio CR", func() {
// given
enableDualStack := true
iop := iopv1alpha1.IstioOperator{
Spec: iopv1alpha1.IstioOperatorSpec{},
}
istioCR := istiov1alpha2.Istio{Spec: istiov1alpha2.IstioSpec{
Config: istiov1alpha2.Config{},
Experimental: &istiov1alpha2.Experimental{
EnableDualStack: &enableDualStack,
},
}}

// when
out, err := istioCR.MergeInto(iop)

valuesMap, err := values.MapFromObject(out.Spec.Values)
Expect(err).ShouldNot(HaveOccurred())

Expect(values.TryGetPathAs[string](valuesMap, "pilot.env.ISTIO_DUAL_STACK")).To(Equal("true"))
})

It("should set dual stack in the mesh Config if dualStack is enabled in the Istio CR", func() {
// given
enableDualStack := true
iop := iopv1alpha1.IstioOperator{
Spec: iopv1alpha1.IstioOperatorSpec{},
}
istioCR := istiov1alpha2.Istio{Spec: istiov1alpha2.IstioSpec{
Config: istiov1alpha2.Config{},
Experimental: &istiov1alpha2.Experimental{
EnableDualStack: &enableDualStack,
},
}}

// when
out, err := istioCR.MergeInto(iop)

meshConfig, err := values.MapFromObject(out.Spec.MeshConfig)
Expect(err).ShouldNot(HaveOccurred())

dualStack, exists := meshConfig.GetPath("defaultConfig.proxyMetadata.ISTIO_DUAL_STACK")
Expect(exists).To(BeTrue())
Expect(dualStack).To(Equal("true"))
})

Context("Pilot", func() {
Context("When Istio CR has 500m configured for CPU limits", func() {
It("should set CPU limits to 500m in IOP", func() {
Expand Down
7 changes: 6 additions & 1 deletion api/v1alpha2/zz_generated.deepcopy.go

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

3 changes: 3 additions & 0 deletions config/crd/bases/operator.kyma-project.io_istios.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1434,6 +1434,9 @@ spec:
experimental:
description: Defines experimental configuration options.
properties:
enableDualStack:
description: Enables the Dual Stack support
type: boolean
pilot:
properties:
enableAlphaGatewayAPI:
Expand Down
7 changes: 4 additions & 3 deletions controllers/istio_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,10 @@ func (r *IstioReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl

r.statusHandler.SetCondition(&istioCR, operatorv1alpha2.NewReasonWithMessage(operatorv1alpha2.ConditionReasonReconcileUnknown))

if err := r.validate(&istioCR); err != nil {
return ctrl.Result{}, r.statusHandler.UpdateToError(ctx, &istioCR, err)
}

istioImages, imgErr := images.GetImages()
if imgErr != nil {
return r.terminateReconciliation(ctx, &istioCR, describederrors.NewDescribedError(imgErr, "Unable to get Istio images environments"),
Expand Down Expand Up @@ -299,9 +303,6 @@ func (r *IstioReconciler) finishReconcile(ctx context.Context, istioCR *operator

r.statusHandler.SetCondition(istioCR, operatorv1alpha2.NewReasonWithMessage(operatorv1alpha2.ConditionReasonReconcileSucceeded))
r.statusHandler.SetCondition(istioCR, operatorv1alpha2.NewReasonWithMessage(operatorv1alpha2.ConditionReasonIngressTargetingUserResourceNotFound))
if err := r.validate(istioCR); err != nil {
return ctrl.Result{}, r.statusHandler.UpdateToError(ctx, istioCR, err)
}

if err := r.statusHandler.UpdateToReady(ctx, istioCR); err != nil {
r.log.Error(err, "Error during updating status to ready")
Expand Down
6 changes: 3 additions & 3 deletions controllers/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@ func (r *IstioReconciler) validate(istioCR *operatorv1alpha2.Istio) describederr
if istioCR.Spec.Experimental != nil {
// user has experimental field applied in their CR
// return error with description
r.log.Info("Experimental features are not supported in this image flavour")
return describederrors.NewDescribedError(errors.New("istio CR contains experimental feature"), "Experimental features are not supported in this image flavour").
SetWarning().
err := errors.New("istio CR contains experimental feature")
r.log.Error(err, "Experimental features are not supported in this image flavour")
return describederrors.NewDescribedError(err, "Experimental features are not supported in this image flavour").
SetCondition(false)
}
return nil
Expand Down
4 changes: 2 additions & 2 deletions controllers/validate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import (
)

var _ = Describe("Istio Controller", func() {
It("should set CR status to warning if experimental fields have been defined", func() {
It("should set CR status to error if experimental fields have been defined", func() {
// given
istioCR := &operatorv1alpha2.Istio{
ObjectMeta: metav1.ObjectMeta{
Expand Down Expand Up @@ -70,7 +70,7 @@ var _ = Describe("Istio Controller", func() {
err = fakeClient.Get(context.Background(), client.ObjectKeyFromObject(istioCR), &updatedIstioCR)
Expect(err).To(Not(HaveOccurred()))

Expect(updatedIstioCR.Status.State).Should(Equal(operatorv1alpha2.Warning))
Expect(updatedIstioCR.Status.State).Should(Equal(operatorv1alpha2.Error))
Expect(updatedIstioCR.Status.Description).To(ContainSubstring("Experimental features are not supported in this image flavour"))
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ spec:
#[...]
```

The experimental features should be only available to be used with a separately built controller image. Using the experimental features with production image should result in setting the Istio rustom resource to the `Warning` state.
The experimental features should be only available to be used with a separately built controller image. Using the experimental features with production image should result in setting the Istio custom resource to the `Error` state and break the reconciliation to avoid unintended changes.

### SAP BTP, Kyma Runtime
In context of SAP BTP, Kyma runtime, experimental features should only be available in the experimental release channel.
Expand Down
3 changes: 3 additions & 0 deletions docs/release-notes/1.24.0.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,8 @@
We simplified the Istio installation, so it doesn't use port forwarding to get the Istio version.
It also doesn't print the message about the Istio downgrade.
See [#1672](https://github.com/kyma-project/istio/pull/1672).
- Added a Dual Stack experimental support
- We've added dual-stack experimental support.
See [#1700](https://github.com/kyma-project/istio/pull/1700).

## Fixed Bugs
1 change: 1 addition & 0 deletions docs/user/04-00-istio-custom-resource.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ This table lists all the possible parameters of Istio CR together with their des
| **config.gatewayExternalTrafficPolicy** | string | Defines the external traffic policy for Istio Ingress Gateway Service. Valid configurations are `Local` or `Cluster`. The external traffic policy set to `Local` preserves the client IP in the request but also introduces the risk of unbalanced traffic distribution. |
| **config.telemetry.metrics.prometheusMerge** | bool | Enables the [prometheusMerge](https://istio.io/latest/docs/ops/integrations/prometheus/#option-1-metrics-merging) feature from Istio, which merges the application's and Istio's metrics and exposes them together at `:15020/stats/prometheus` for scraping using plain HTTP. Updating the field causes a restart of the Istio sidecar proxies. |
| **experimental** | object | Defines additional experimental features that can be enabled in experimental builds. |
| **experimental.enableDualStack** | bool | Enables dual-stack support. |
| **experimental.pilot** | object | Defines additional experimental features that can be enabled in Istio pilot component. |
| **experimental.pilot.enableAlphaGatewayAPI** | bool | Enables support for alpha Kubernetes Gateway API. |
| **experimental.pilot.enableMultiNetworkDiscoverGatewayAPI** | bool | Enables support for multi-network discovery in Kubernetes Gateway API. |
Expand Down
27 changes: 26 additions & 1 deletion internal/clusterconfig/clusterconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,9 +140,14 @@ const (
loadBalancerNlbTargetType = "instance"
loadBalancerTypeAnnotation = "service.beta.kubernetes.io/aws-load-balancer-type"
loadBalancerType = "nlb"
loadBalancerDualStackAnnotation = "service.beta.kubernetes.io/aws-load-balancer-ip-address-type"
loadBalancerDualStack = "dualstack"

loadBalancerProxyProtocolOpenStackAnnotation = "loadbalancer.openstack.org/proxy-protocol"
loadBalancerProxyProtocolOpenStack = "v1"

istioIngressServiceName = "istio-ingressgateway"
istioIngressNamespace = "istio-system"
)

func ShouldUseNLB(ctx context.Context, k8sClient client.Client) (bool, error) {
Expand All @@ -156,7 +161,7 @@ func ShouldUseNLB(ctx context.Context, k8sClient client.Client) (bool, error) {
}

var ingressGatewaySvc corev1.Service
err = k8sClient.Get(ctx, client.ObjectKey{Namespace: "istio-system", Name: "istio-ingressgateway"}, &ingressGatewaySvc)
err = k8sClient.Get(ctx, client.ObjectKey{Namespace: istioIngressNamespace, Name: istioIngressServiceName}, &ingressGatewaySvc)
if err != nil {
if errors.IsNotFound(err) {
return false, nil
Expand All @@ -171,6 +176,26 @@ func ShouldUseNLB(ctx context.Context, k8sClient client.Client) (bool, error) {
return false, nil
}

// IsDualStack checks whether the Ingress service has an IP address type annotation set to 'dualstack'
// This annotation is set automatically by 'gardener-extension-provider-aws' on the Gardener side
// if the cluster infrastructure supports IPv6
func IsDualStack(ctx context.Context, k8sClient client.Client) (bool, error) {
var ingressGatewaySvc corev1.Service
err := k8sClient.Get(ctx, client.ObjectKey{Namespace: istioIngressNamespace, Name: istioIngressServiceName}, &ingressGatewaySvc)
if err != nil {
if errors.IsNotFound(err) {
return false, nil
}
return false, err
}

if value, ok := ingressGatewaySvc.Annotations[loadBalancerDualStackAnnotation]; ok && value == loadBalancerDualStack {
return true, nil
}

return false, nil
}

// awsConfig returns config specific to AWS cluster.
// The function evaluates whether to use NLB or ELB load balancer, based on:
//
Expand Down
Loading
Loading