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

[release-1.12] Update istio gateway from v1alpha3 to v1beta1 #1647

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: 2 additions & 2 deletions pkg/apis/operator/base/ingressconfiguration.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ limitations under the License.
package base

import (
istiov1alpha3 "istio.io/api/networking/v1alpha3"
istiov1beta1 "istio.io/api/networking/v1beta1"
v1 "k8s.io/api/core/v1"
)

Expand Down Expand Up @@ -65,5 +65,5 @@ type IstioGatewayOverride struct {
Selector map[string]string `json:"selector,omitempty"`

// A list of server specifications.
Servers []*istiov1alpha3.Server `json:"servers,omitempty"`
Servers []*istiov1beta1.Server `json:"servers,omitempty"`
}
4 changes: 2 additions & 2 deletions pkg/apis/operator/base/zz_generated.deepcopy.go

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

27 changes: 21 additions & 6 deletions pkg/reconciler/knativeserving/ingress/istio.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,11 @@ package ingress

import (
"context"
"strings"

mf "github.com/manifestival/manifestival"
"go.uber.org/zap"
istionetworkingv1alpha3 "istio.io/client-go/pkg/apis/networking/v1alpha3"
istionetworkingv1beta "istio.io/client-go/pkg/apis/networking/v1beta1"
"istio.io/client-go/pkg/clientset/versioned/scheme"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"knative.dev/operator/pkg/apis/operator/base"
Expand All @@ -38,9 +39,19 @@ func istioTransformers(ctx context.Context, instance *v1beta1.KnativeServing) []
func gatewayTransform(instance *servingv1beta1.KnativeServing, log *zap.SugaredLogger) mf.Transformer {
return func(u *unstructured.Unstructured) error {
// Update the deployment with the new registry and tag
if u.GetAPIVersion() == "networking.istio.io/v1alpha3" && u.GetKind() == "Gateway" {
gateway := &istionetworkingv1alpha3.Gateway{}
if err := scheme.Scheme.Convert(u, gateway, nil); err != nil {
if u.GetKind() == "Gateway" {
if !strings.HasPrefix(u.GetAPIVersion(), "networking.istio.io/") {
return nil
}
beta := true
if strings.HasSuffix(u.GetAPIVersion(), "v1alpha3") {
u.SetAPIVersion("networking.istio.io/v1beta1")
beta = false
}

gateway := &istionetworkingv1beta.Gateway{}
err := scheme.Scheme.Convert(u, gateway, nil)
if err != nil {
return err
}

Expand All @@ -50,7 +61,7 @@ func gatewayTransform(instance *servingv1beta1.KnativeServing, log *zap.SugaredL
}
}
// TODO: cluster-local-gateway was removed since v0.20 https://github.com/knative-extensions/net-istio/commit/058432d749435ef1fc61aa2b1fd048d0c75460ee
// Reomove it once operator stops v0.20 support.
// Remove it once operator stops v0.20 support.
if u.GetName() == "cluster-local-gateway" || u.GetName() == "knative-local-gateway" {
if err := updateIstioGateway(localGateway(instance), gateway, log); err != nil {
return err
Expand All @@ -60,6 +71,10 @@ func gatewayTransform(instance *servingv1beta1.KnativeServing, log *zap.SugaredL
if err := scheme.Scheme.Convert(gateway, u, nil); err != nil {
return err
}

if !beta {
u.SetAPIVersion("networking.istio.io/v1alpha3")
}
}
return nil
}
Expand All @@ -79,7 +94,7 @@ func localGateway(instance *servingv1beta1.KnativeServing) *base.IstioGatewayOve
return nil
}

func updateIstioGateway(override *base.IstioGatewayOverride, gateway *istionetworkingv1alpha3.Gateway, log *zap.SugaredLogger) error {
func updateIstioGateway(override *base.IstioGatewayOverride, gateway *istionetworkingv1beta.Gateway, log *zap.SugaredLogger) error {
if override != nil && len(override.Selector) > 0 {
log.Debugw("Updating Gateway", "name", gateway.GetName(), "gatewayOverrides", override)
gateway.Spec.Selector = override.Selector
Expand Down
156 changes: 148 additions & 8 deletions pkg/reconciler/knativeserving/ingress/istio_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@ import (

"go.uber.org/zap"
istiov1alpha3 "istio.io/api/networking/v1alpha3"
istiov1beta1 "istio.io/api/networking/v1beta1"
istionetworkingv1alpha3 "istio.io/client-go/pkg/apis/networking/v1alpha3"
istionetworkingv1beta1 "istio.io/client-go/pkg/apis/networking/v1beta1"
"istio.io/client-go/pkg/clientset/versioned/scheme"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"knative.dev/operator/pkg/apis/operator/base"
Expand All @@ -31,14 +33,14 @@ import (

var log = zap.NewNop().Sugar()

func gatewayOverride(selector map[string]string, servers []*istiov1alpha3.Server) *base.IstioGatewayOverride {
func gatewayOverride(selector map[string]string, servers []*istiov1beta1.Server) *base.IstioGatewayOverride {
return &base.IstioGatewayOverride{
Selector: selector,
Servers: servers,
}
}

func TestGatewayTransform(t *testing.T) {
func TestGatewayTransformV1alpha3(t *testing.T) {
serverIn := []*istiov1alpha3.Server{
{
Hosts: []string{"localhost"},
Expand All @@ -48,13 +50,13 @@ func TestGatewayTransform(t *testing.T) {
Port: &istiov1alpha3.Port{Name: "test"},
}}

serverUpdate := []*istiov1alpha3.Server{
serverUpdate := []*istiov1beta1.Server{
{
Hosts: []string{"localhost-1"},
Port: &istiov1alpha3.Port{Name: "test-1", Protocol: "proto-1", Number: 25, TargetPort: 53},
Port: &istiov1beta1.Port{Name: "test-1", Protocol: "proto-1", Number: 25, TargetPort: 53},
}, {
Hosts: []string{"localhost-1"},
Port: &istiov1alpha3.Port{Name: "test-1", Protocol: "proto-2", Number: 45, TargetPort: 23},
Port: &istiov1beta1.Port{Name: "test-1", Protocol: "proto-2", Number: 45, TargetPort: 23},
}}

tests := []struct {
Expand All @@ -67,7 +69,7 @@ func TestGatewayTransform(t *testing.T) {
deprecatedKnativeIngressGateway base.IstioGatewayOverride
deprecatedClusterLocalGateway base.IstioGatewayOverride
expected map[string]string
expectedServersIn []*istiov1alpha3.Server
expectedServersIn []*istiov1beta1.Server
}{{
name: "update ingress gateway",
gatewayName: "knative-ingress-gateway",
Expand Down Expand Up @@ -132,7 +134,7 @@ func TestGatewayTransform(t *testing.T) {

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
gateway := makeUnstructuredGateway(t, tt.gatewayName, tt.in, tt.serversIn)
gateway := makeUnstructuredGatewayAlpha(tt.gatewayName, tt.in, tt.serversIn)
instance := &servingv1beta1.KnativeServing{
Spec: servingv1beta1.KnativeServingSpec{
Ingress: &servingv1beta1.IngressConfigs{
Expand Down Expand Up @@ -162,7 +164,145 @@ func TestGatewayTransform(t *testing.T) {
}
}

func makeUnstructuredGateway(t *testing.T, name string, selector map[string]string, servers []*istiov1alpha3.Server) *unstructured.Unstructured {
func TestGatewayTransform(t *testing.T) {
serverIn := []*istiov1beta1.Server{
{
Hosts: []string{"localhost"},
Port: &istiov1beta1.Port{Name: "test"},
}, {
Hosts: []string{"localhost"},
Port: &istiov1beta1.Port{Name: "test"},
}}

serverUpdate := []*istiov1beta1.Server{
{
Hosts: []string{"localhost-1"},
Port: &istiov1beta1.Port{Name: "test-1", Protocol: "proto-1", Number: 25, TargetPort: 53},
}, {
Hosts: []string{"localhost-1"},
Port: &istiov1beta1.Port{Name: "test-1", Protocol: "proto-2", Number: 45, TargetPort: 23},
}}

tests := []struct {
name string
gatewayName string
in map[string]string
serversIn []*istiov1beta1.Server
knativeIngressGateway *base.IstioGatewayOverride
clusterLocalGateway *base.IstioGatewayOverride
deprecatedKnativeIngressGateway base.IstioGatewayOverride
deprecatedClusterLocalGateway base.IstioGatewayOverride
expected map[string]string
expectedServersIn []*istiov1beta1.Server
}{{
name: "update ingress gateway",
gatewayName: "knative-ingress-gateway",
in: map[string]string{
"istio": "old-istio",
},
serversIn: serverIn,
knativeIngressGateway: gatewayOverride(map[string]string{"istio": "knative-ingress"}, serverUpdate),
clusterLocalGateway: gatewayOverride(map[string]string{"istio": "cluster-local"}, nil),
expected: map[string]string{
"istio": "knative-ingress",
},
expectedServersIn: serverUpdate,
}, {
name: "update local gateway",
gatewayName: "cluster-local-gateway",
in: map[string]string{
"istio": "old-istio",
},
knativeIngressGateway: gatewayOverride(map[string]string{"istio": "knative-ingress"}, nil),
clusterLocalGateway: gatewayOverride(map[string]string{"istio": "cluster-local"}, serverUpdate),
expected: map[string]string{
"istio": "cluster-local",
},
expectedServersIn: serverUpdate,
}, {
name: "update ingress gateway with both new and deprecate config",
gatewayName: "knative-ingress-gateway",
in: map[string]string{
"istio": "old-istio",
},
knativeIngressGateway: gatewayOverride(map[string]string{"istio": "win"}, nil),
deprecatedKnativeIngressGateway: *gatewayOverride(map[string]string{"istio": "lose"}, nil),
expected: map[string]string{
"istio": "win",
},
}, {
name: "update local gateway with both new and deprecate config",
gatewayName: "cluster-local-gateway",
in: map[string]string{
"istio": "old-istio",
},
clusterLocalGateway: gatewayOverride(map[string]string{"istio": "win"}, nil),
deprecatedClusterLocalGateway: *gatewayOverride(map[string]string{"istio": "lose"}, nil),
expected: map[string]string{
"istio": "win",
},
}, {
name: "do not update unknown gateway",
gatewayName: "not-knative-ingress-gateway",
in: map[string]string{
"istio": "old-istio",
},
knativeIngressGateway: gatewayOverride(map[string]string{"istio": "knative-ingress"}, nil),
clusterLocalGateway: gatewayOverride(map[string]string{"istio": "cluster-local"}, nil),
deprecatedKnativeIngressGateway: *gatewayOverride(map[string]string{"istio": "lose"}, nil),
deprecatedClusterLocalGateway: *gatewayOverride(map[string]string{"istio": "cluster-local"}, nil),
expected: map[string]string{
"istio": "old-istio",
},
}}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
gateway := makeUnstructuredGateway(tt.gatewayName, tt.in, tt.serversIn)
instance := &servingv1beta1.KnativeServing{
Spec: servingv1beta1.KnativeServingSpec{
Ingress: &servingv1beta1.IngressConfigs{
Istio: base.IstioIngressConfiguration{
Enabled: true,
KnativeIngressGateway: tt.knativeIngressGateway,
KnativeLocalGateway: tt.clusterLocalGateway,
},
},
},
}

gatewayTransform(instance, log)(gateway)

gatewayResult := &istionetworkingv1beta1.Gateway{}
err := scheme.Scheme.Convert(gateway, gatewayResult, nil)
util.AssertEqual(t, err, nil)
util.AssertDeepEqual(t, gatewayResult.Spec.Selector, tt.expected)
for i, server := range gatewayResult.Spec.Servers {
util.AssertDeepEqual(t, server.Hosts, tt.expectedServersIn[i].Hosts)
util.AssertDeepEqual(t, server.Port.Name, tt.expectedServersIn[i].Port.Name)
util.AssertDeepEqual(t, server.Port.Number, tt.expectedServersIn[i].Port.Number)
util.AssertDeepEqual(t, server.Port.TargetPort, tt.expectedServersIn[i].Port.TargetPort)
util.AssertDeepEqual(t, server.Port.Protocol, tt.expectedServersIn[i].Port.Protocol)
}
})
}
}

func makeUnstructuredGateway(name string, selector map[string]string, servers []*istiov1beta1.Server) *unstructured.Unstructured {
gateway := &istionetworkingv1beta1.Gateway{}
result := &unstructured.Unstructured{}
gateway.SetName(name)
gateway.Spec.Selector = selector
gateway.Spec.Servers = servers

if err := scheme.Scheme.Convert(gateway, result, nil); err != nil {
panic(err)
}

return result
}

func makeUnstructuredGatewayAlpha(name string, selector map[string]string, servers []*istiov1alpha3.Server) *unstructured.Unstructured {
gateway := &istionetworkingv1alpha3.Gateway{}
result := &unstructured.Unstructured{}
gateway.SetName(name)
Expand Down
Loading