Skip to content

Commit

Permalink
Revert "make "preview" standard prefix name for bluegreen"
Browse files Browse the repository at this point in the history
This reverts commit 50c0ce5.

Signed-off-by: nbn01 <nandan_bn@intuit.com>
  • Loading branch information
nbn01 committed Mar 31, 2022
1 parent 50c0ce5 commit c36fb50
Show file tree
Hide file tree
Showing 8 changed files with 70 additions and 60 deletions.
11 changes: 7 additions & 4 deletions admiral/cmd/admiral/cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,16 @@ import (
"context"
"flag"
"fmt"
"os"
"os/signal"
"syscall"
"time"

"github.com/istio-ecosystem/admiral/admiral/pkg/apis/admiral/routes"
"github.com/istio-ecosystem/admiral/admiral/pkg/apis/admiral/server"
"github.com/istio-ecosystem/admiral/admiral/pkg/clusters"
"github.com/istio-ecosystem/admiral/admiral/pkg/controller/common"
log "github.com/sirupsen/logrus"
"os"
"os/signal"
"syscall"
"time"

"github.com/spf13/cobra"
)
Expand Down Expand Up @@ -103,6 +104,8 @@ func GetRootCmd(args []string) *cobra.Command {
"The label value, on a namespace, which tells Istio to perform sidecar injection")
rootCmd.PersistentFlags().StringVar(&params.HostnameSuffix, "hostname_suffix", "global",
"The hostname suffix to customize the cname generated by admiral. Default suffix value will be \"global\"")
rootCmd.PersistentFlags().StringVar(&params.PreviewHostnamePrefix, "preview_hostname_prefix", "preview",
"The hostname prefix to customize the cname generated by admiral for bluegreen rollout preview service. Default suffix value will be \"preview\"")
rootCmd.PersistentFlags().StringVar(&params.LabelSet.WorkloadIdentityKey, "workload_identity_key", "identity",
"The workload identity key, on deployment which holds identity value used to generate cname by admiral. Default label key will be \"identity\" Admiral will look for a label with this key. If present, that will be used. If not, it will try an annotation (for use cases where an identity is longer than 63 chars)")
rootCmd.PersistentFlags().StringVar(&params.LabelSet.GlobalTrafficDeploymentLabel, "globaltraffic_deployment_label", "identity",
Expand Down
36 changes: 17 additions & 19 deletions admiral/pkg/clusters/registry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,9 @@ package clusters

import (
"context"
"strings"
"testing"
"time"

"github.com/google/go-cmp/cmp"
depModel "github.com/istio-ecosystem/admiral/admiral/pkg/apis/admiral/model"
v1 "github.com/istio-ecosystem/admiral/admiral/pkg/apis/admiral/v1"
"github.com/istio-ecosystem/admiral/admiral/pkg/apis/admiral/v1"
"github.com/istio-ecosystem/admiral/admiral/pkg/controller/admiral"
"github.com/istio-ecosystem/admiral/admiral/pkg/controller/common"
"github.com/istio-ecosystem/admiral/admiral/pkg/test"
Expand All @@ -21,6 +17,9 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/rest"
"k8s.io/client-go/tools/clientcmd"
"strings"
"testing"
"time"
)

func init() {
Expand All @@ -37,7 +36,6 @@ func init() {
SecretResolver: "",
WorkloadSidecarUpdate: "enabled",
WorkloadSidecarName: "default",
PreviewHostnamePrefix: "preview",
}

p.LabelSet.WorkloadIdentityKey = "identity"
Expand Down Expand Up @@ -451,24 +449,24 @@ func TestUpdateCacheController(t *testing.T) {

//Struct of test case info. Name is required.
testCases := []struct {
name string
oldConfig *rest.Config
newConfig *rest.Config
clusterId string
name string
oldConfig *rest.Config
newConfig *rest.Config
clusterId string
shouldRefresh bool
}{
{
name: "Should update controller when kubeconfig changes",
oldConfig: originalConfig,
newConfig: changedConfig,
clusterId: "test.cluster",
name: "Should update controller when kubeconfig changes",
oldConfig: originalConfig,
newConfig: changedConfig,
clusterId: "test.cluster",
shouldRefresh: true,
},
{
name: "Should not update controller when kubeconfig doesn't change",
oldConfig: originalConfig,
newConfig: originalConfig,
clusterId: "test.cluster",
name: "Should not update controller when kubeconfig doesn't change",
oldConfig: originalConfig,
newConfig: originalConfig,
clusterId: "test.cluster",
shouldRefresh: false,
},
}
Expand All @@ -478,7 +476,7 @@ func TestUpdateCacheController(t *testing.T) {
t.Run(c.name, func(t *testing.T) {
hook := logTest.NewGlobal()
rr.RemoteControllers[c.clusterId].ApiServer = c.oldConfig.Host
d, err := admiral.NewDeploymentController(make(chan struct{}), &test.MockDeploymentHandler{}, c.oldConfig, time.Second*time.Duration(300))
d, err := admiral.NewDeploymentController(make(chan struct{}), &test.MockDeploymentHandler{}, c.oldConfig, time.Second*time.Duration(300))
if err != nil {
t.Fatalf("Unexpected error creating controller %v", err)
}
Expand Down
4 changes: 2 additions & 2 deletions admiral/pkg/clusters/serviceentry.go
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ func modifyServiceEntryForNewServiceOrPod(event admiral.EventType, env string, s
activeServiceName := sourceRollouts[sourceCluster].Spec.Strategy.BlueGreen.ActiveService
previewServiceName := sourceRollouts[sourceCluster].Spec.Strategy.BlueGreen.PreviewService
oldPorts := ep.Ports
if previewService, ok := weightedServices[previewServiceName]; strings.HasPrefix(key, common.BlueGreenRolloutPreviewPrefix+common.Sep) && ok {
if previewService, ok := weightedServices[previewServiceName]; strings.HasPrefix(key, common.GetPreviewHostnamePrefix()+common.Sep) && ok {
previewServiceInstance := previewService.Service
localFqdn := previewServiceInstance.Name + common.Sep + previewServiceInstance.Namespace + common.DotLocalDomainSuffix
cnames[localFqdn] = "1"
Expand Down Expand Up @@ -603,7 +603,7 @@ func createServiceEntryForRollout(event admiral.EventType, rc *RemoteController,
if destRollout.Spec.Strategy.BlueGreen != nil && destRollout.Spec.Strategy.BlueGreen.PreviewService != "" {
rolloutServices := getServiceForRollout(rc, destRollout)
if _, ok := rolloutServices[destRollout.Spec.Strategy.BlueGreen.PreviewService]; ok {
previewGlobalFqdn := common.BlueGreenRolloutPreviewPrefix + common.Sep + common.GetCnameForRollout(destRollout, workloadIdentityKey, common.GetHostnameSuffix())
previewGlobalFqdn := common.GetPreviewHostnamePrefix() + common.Sep + common.GetCnameForRollout(destRollout, workloadIdentityKey, common.GetHostnameSuffix())
previewAddress := getUniqueAddress(admiralCache, previewGlobalFqdn)
if len(previewGlobalFqdn) != 0 && len(previewAddress) != 0 {
generateServiceEntry(event, admiralCache, meshPorts, previewGlobalFqdn, rc, serviceEntries, previewAddress, san)
Expand Down
1 change: 1 addition & 0 deletions admiral/pkg/clusters/serviceentry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1006,6 +1006,7 @@ func TestCreateServiceEntryForBlueGreenRolloutsUsecase(t *testing.T) {
PreviewHostnamePrefix: "preview",
}
rr, _ := InitAdmiral(context.Background(), p)
t.Logf(common.GetPreviewHostnamePrefix())
config := rest.Config{
Host: "localhost",
}
Expand Down
56 changes: 27 additions & 29 deletions admiral/pkg/controller/common/common.go
Original file line number Diff line number Diff line change
@@ -1,39 +1,37 @@
package common

import (
"sort"
"strings"

v1 "github.com/istio-ecosystem/admiral/admiral/pkg/apis/admiral/v1"
"github.com/istio-ecosystem/admiral/admiral/pkg/apis/admiral/v1"
log "github.com/sirupsen/logrus"
k8sAppsV1 "k8s.io/api/apps/v1"
k8sV1 "k8s.io/api/core/v1"
"sort"
"strings"
)

const (
NamespaceKubeSystem = "kube-system"
NamespaceIstioSystem = "istio-system"
Env = "env"
Http = "http"
Grpc = "grpc"
GrpcWeb = "grpc-web"
Http2 = "http2"
DefaultMtlsPort = 15443
DefaultServiceEntryPort = 80
Sep = "."
Dash = "-"
Slash = "/"
DotLocalDomainSuffix = ".svc.cluster.local"
Mesh = "mesh"
MulticlusterIngressGateway = "istio-multicluster-ingressgateway"
LocalAddressPrefix = "240.0"
NodeRegionLabel = "failure-domain.beta.kubernetes.io/region"
SpiffePrefix = "spiffe://"
SidecarEnabledPorts = "traffic.sidecar.istio.io/includeInboundPorts"
Default = "default"
AdmiralIgnoreAnnotation = "admiral.io/ignore"
AdmiralCnameCaseSensitive = "admiral.io/cname-case-sensitive"
BlueGreenRolloutPreviewPrefix = "preview"
NamespaceKubeSystem = "kube-system"
NamespaceIstioSystem = "istio-system"
Env = "env"
Http = "http"
Grpc = "grpc"
GrpcWeb = "grpc-web"
Http2 = "http2"
DefaultMtlsPort = 15443
DefaultServiceEntryPort = 80
Sep = "."
Dash = "-"
Slash = "/"
DotLocalDomainSuffix = ".svc.cluster.local"
Mesh = "mesh"
MulticlusterIngressGateway = "istio-multicluster-ingressgateway"
LocalAddressPrefix = "240.0"
NodeRegionLabel = "failure-domain.beta.kubernetes.io/region"
SpiffePrefix = "spiffe://"
SidecarEnabledPorts = "traffic.sidecar.istio.io/includeInboundPorts"
Default = "default"
AdmiralIgnoreAnnotation = "admiral.io/ignore"
AdmiralCnameCaseSensitive = "admiral.io/cname-case-sensitive"
)

type Event int
Expand Down Expand Up @@ -89,7 +87,7 @@ func GetCname(deployment *k8sAppsV1.Deployment, identifier string, nameSuffix st
return strings.ToLower(cname)
}

func GetCnameVal(vals []string) string {
func GetCnameVal(vals[] string) string {
return strings.Join(vals, Sep)
}

Expand Down Expand Up @@ -252,4 +250,4 @@ func GetGtpEnv(gtp *v1.GlobalTrafficPolicy) string {
environment = Default
}
return environment
}
}
12 changes: 7 additions & 5 deletions admiral/pkg/controller/common/common_test.go
Original file line number Diff line number Diff line change
@@ -1,16 +1,17 @@
package common

import (
"reflect"
"strings"
"testing"
"time"

"github.com/google/go-cmp/cmp"
"github.com/google/go-cmp/cmp/cmpopts"
v12 "github.com/istio-ecosystem/admiral/admiral/pkg/apis/admiral/v1"
k8sAppsV1 "k8s.io/api/apps/v1"
k8sCoreV1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1"
"reflect"
"strings"
"testing"
"time"
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

var ignoreUnexported = cmpopts.IgnoreUnexported(v12.GlobalTrafficPolicy{}.Status)
Expand All @@ -29,6 +30,7 @@ func init() {
SecretResolver: "",
WorkloadSidecarName: "default",
WorkloadSidecarUpdate: "disabled",
PreviewHostnamePrefix: "preview",
}

p.LabelSet.WorkloadIdentityKey = "identity"
Expand Down
7 changes: 6 additions & 1 deletion admiral/pkg/controller/common/config.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
package common

import (
log "github.com/sirupsen/logrus"
"sync"
"time"

log "github.com/sirupsen/logrus"
)

var admiralParams = AdmiralParams{
Expand Down Expand Up @@ -71,6 +72,10 @@ func GetHostnameSuffix() string {
return admiralParams.HostnameSuffix
}

func GetPreviewHostnamePrefix() string {
return admiralParams.PreviewHostnamePrefix
}

func GetWorkloadIdentifier() string {
return admiralParams.LabelSet.WorkloadIdentityKey
}
Expand Down
3 changes: 3 additions & 0 deletions admiral/pkg/controller/common/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,9 @@ func TestConfigManagement(t *testing.T) {
if GetHostnameSuffix() != "mesh" {
t.Errorf("Hostname suffix mismatch, expected mesh, got %v", GetHostnameSuffix())
}
if GetPreviewHostnamePrefix() != "preview" {
t.Errorf("Prefix for preview hostname mismatch, expected preview, got %v", GetPreviewHostnamePrefix())
}
if GetSyncNamespace() != "ns" {
t.Errorf("Sync namespace mismatch, expected ns, got %v", GetSyncNamespace())
}
Expand Down

0 comments on commit c36fb50

Please sign in to comment.