Skip to content

Commit 7c77744

Browse files
huikanghuikang
authored
fix: change virtualService to pointer (argoproj#1558)
Signed-off-by: Hui Kang <[email protected]>
1 parent c7e8084 commit 7c77744

File tree

11 files changed

+34
-24
lines changed

11 files changed

+34
-24
lines changed

‎pkg/apis/rollouts/v1alpha1/types.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -374,7 +374,7 @@ type NginxTrafficRouting struct {
374374
// IstioTrafficRouting configuration for Istio service mesh to enable fine grain configuration
375375
type IstioTrafficRouting struct {
376376
// VirtualService references an Istio VirtualService to modify to shape traffic
377-
VirtualService IstioVirtualService `json:"virtualService,omitempty" protobuf:"bytes,1,opt,name=virtualService"`
377+
VirtualService *IstioVirtualService `json:"virtualService,omitempty" protobuf:"bytes,1,opt,name=virtualService"`
378378
// DestinationRule references an Istio DestinationRule to modify to shape traffic
379379
DestinationRule *IstioDestinationRule `json:"destinationRule,omitempty" protobuf:"bytes,2,opt,name=destinationRule"`
380380
// VirtualServices references a list of Istio VirtualService to modify to shape traffic

‎pkg/apis/rollouts/v1alpha1/zz_generated.deepcopy.go

Lines changed: 5 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎pkg/apis/rollouts/validation/validation_references.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -225,11 +225,11 @@ func ValidateRolloutVirtualServicesConfig(r *v1alpha1.Rollout) error {
225225
canary := r.Spec.Strategy.Canary
226226
if canary.TrafficRouting != nil && canary.TrafficRouting.Istio != nil {
227227
if istioutil.MultipleVirtualServiceConfigured(r) {
228-
if r.Spec.Strategy.Canary.TrafficRouting.Istio.VirtualService.Name != "" {
228+
if r.Spec.Strategy.Canary.TrafficRouting.Istio.VirtualService != nil {
229229
return field.InternalError(fldPath, fmt.Errorf(errorString))
230230
}
231231
} else {
232-
if r.Spec.Strategy.Canary.TrafficRouting.Istio.VirtualService.Name == "" {
232+
if r.Spec.Strategy.Canary.TrafficRouting.Istio.VirtualService == nil {
233233
return field.InternalError(fldPath, fmt.Errorf(errorString))
234234
}
235235
}
@@ -249,7 +249,7 @@ func ValidateVirtualService(rollout *v1alpha1.Rollout, obj unstructured.Unstruct
249249
virtualServices = rollout.Spec.Strategy.Canary.TrafficRouting.Istio.VirtualServices
250250
} else {
251251
fldPath = field.NewPath("spec", "strategy", "canary", "trafficRouting", "istio", "virtualService", "name")
252-
virtualServices = []v1alpha1.IstioVirtualService{rollout.Spec.Strategy.Canary.TrafficRouting.Istio.VirtualService}
252+
virtualServices = []v1alpha1.IstioVirtualService{*rollout.Spec.Strategy.Canary.TrafficRouting.Istio.VirtualService}
253253
}
254254

255255
virtualServiceRecordName := obj.GetName()

‎pkg/apis/rollouts/validation/validation_references_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -412,7 +412,7 @@ func TestValidateVirtualService(t *testing.T) {
412412
CanaryService: "canary",
413413
TrafficRouting: &v1alpha1.RolloutTrafficRouting{
414414
Istio: &v1alpha1.IstioTrafficRouting{
415-
VirtualService: v1alpha1.IstioVirtualService{
415+
VirtualService: &v1alpha1.IstioVirtualService{
416416
Name: "istio-vsvc",
417417
Routes: []string{
418418
"primary",
@@ -520,7 +520,7 @@ func TestValidateRolloutVirtualServicesConfig(t *testing.T) {
520520

521521
ro.Spec.Strategy.Canary.TrafficRouting = &v1alpha1.RolloutTrafficRouting{
522522
Istio: &v1alpha1.IstioTrafficRouting{
523-
VirtualService: v1alpha1.IstioVirtualService{
523+
VirtualService: &v1alpha1.IstioVirtualService{
524524
Name: "istio-vsvc1-name",
525525
},
526526
VirtualServices: []v1alpha1.IstioVirtualService{{Name: "istio-vsvc1-name", Routes: nil}},
@@ -537,7 +537,7 @@ func TestValidateRolloutVirtualServicesConfig(t *testing.T) {
537537

538538
ro.Spec.Strategy.Canary.TrafficRouting = &v1alpha1.RolloutTrafficRouting{
539539
Istio: &v1alpha1.IstioTrafficRouting{
540-
VirtualService: v1alpha1.IstioVirtualService{
540+
VirtualService: &v1alpha1.IstioVirtualService{
541541
Name: "istio-vsvc1-name",
542542
},
543543
},

‎pkg/apis/rollouts/validation/validation_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -469,7 +469,7 @@ func TestCanaryExperimentStepWithWeight(t *testing.T) {
469469
invalidRo := ro.DeepCopy()
470470
invalidRo.Spec.Strategy.Canary.TrafficRouting = &v1alpha1.RolloutTrafficRouting{
471471
Istio: &v1alpha1.IstioTrafficRouting{
472-
VirtualService: v1alpha1.IstioVirtualService{
472+
VirtualService: &v1alpha1.IstioVirtualService{
473473
Name: "virtualSvc",
474474
},
475475
},

‎rollout/trafficrouting/istio/controller.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,7 @@ func (c *IstioController) GetReferencedVirtualServices(ro *v1alpha1.Rollout) (*[
188188
vsvcs = canary.TrafficRouting.Istio.VirtualServices
189189
fldPath = field.NewPath("spec", "strategy", "canary", "trafficRouting", "istio", "virtualServices", "name")
190190
} else {
191-
vsvcs = []v1alpha1.IstioVirtualService{canary.TrafficRouting.Istio.VirtualService}
191+
vsvcs = []v1alpha1.IstioVirtualService{*canary.TrafficRouting.Istio.VirtualService}
192192
fldPath = field.NewPath("spec", "strategy", "canary", "trafficRouting", "istio", "virtualService", "name")
193193
}
194194

‎rollout/trafficrouting/istio/controller_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ func TestGetReferencedVirtualServices(t *testing.T) {
6363
}
6464
ro.Spec.Strategy.Canary.TrafficRouting = &v1alpha1.RolloutTrafficRouting{
6565
Istio: &v1alpha1.IstioTrafficRouting{
66-
VirtualService: v1alpha1.IstioVirtualService{
66+
VirtualService: &v1alpha1.IstioVirtualService{
6767
Name: "istio-vsvc-name",
6868
},
6969
},
@@ -137,7 +137,7 @@ func TestSyncDestinationRule(t *testing.T) {
137137
Canary: &v1alpha1.CanaryStrategy{
138138
TrafficRouting: &v1alpha1.RolloutTrafficRouting{
139139
Istio: &v1alpha1.IstioTrafficRouting{
140-
VirtualService: v1alpha1.IstioVirtualService{
140+
VirtualService: &v1alpha1.IstioVirtualService{
141141
Name: "istio-vsvc",
142142
},
143143
DestinationRule: &v1alpha1.IstioDestinationRule{

‎rollout/trafficrouting/istio/istio.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -532,7 +532,7 @@ func (r *Reconciler) SetWeight(desiredWeight int32, additionalDestinations ...v1
532532
if istioutil.MultipleVirtualServiceConfigured(r.rollout) {
533533
virtualServices = r.rollout.Spec.Strategy.Canary.TrafficRouting.Istio.VirtualServices
534534
} else {
535-
virtualServices = []v1alpha1.IstioVirtualService{r.rollout.Spec.Strategy.Canary.TrafficRouting.Istio.VirtualService}
535+
virtualServices = []v1alpha1.IstioVirtualService{*r.rollout.Spec.Strategy.Canary.TrafficRouting.Istio.VirtualService}
536536
}
537537

538538
for _, virtualService := range virtualServices {

‎rollout/trafficrouting/istio/istio_test.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ func rollout(stableSvc, canarySvc string, istioVirtualService *v1alpha1.IstioVir
5454
CanaryService: canarySvc,
5555
TrafficRouting: &v1alpha1.RolloutTrafficRouting{
5656
Istio: &v1alpha1.IstioTrafficRouting{
57-
VirtualService: *istioVirtualService,
57+
VirtualService: istioVirtualService,
5858
},
5959
},
6060
},
@@ -821,7 +821,7 @@ func TestValidateHTTPRoutes(t *testing.T) {
821821
CanaryService: "canary",
822822
TrafficRouting: &v1alpha1.RolloutTrafficRouting{
823823
Istio: &v1alpha1.IstioTrafficRouting{
824-
VirtualService: v1alpha1.IstioVirtualService{
824+
VirtualService: &v1alpha1.IstioVirtualService{
825825
Routes: routes,
826826
},
827827
},
@@ -872,7 +872,7 @@ func TestValidateTLSRoutes(t *testing.T) {
872872
CanaryService: "canary",
873873
TrafficRouting: &v1alpha1.RolloutTrafficRouting{
874874
Istio: &v1alpha1.IstioTrafficRouting{
875-
VirtualService: v1alpha1.IstioVirtualService{
875+
VirtualService: &v1alpha1.IstioVirtualService{
876876
Routes: routes,
877877
TLSRoutes: tlsRoutes,
878878
},
@@ -990,7 +990,7 @@ func TestValidateHTTPRoutesSubsets(t *testing.T) {
990990
Canary: &v1alpha1.CanaryStrategy{
991991
TrafficRouting: &v1alpha1.RolloutTrafficRouting{
992992
Istio: &v1alpha1.IstioTrafficRouting{
993-
VirtualService: v1alpha1.IstioVirtualService{
993+
VirtualService: &v1alpha1.IstioVirtualService{
994994
Routes: []string{"primary"},
995995
},
996996
DestinationRule: &v1alpha1.IstioDestinationRule{
@@ -1056,7 +1056,7 @@ func rolloutWithDestinationRule() *v1alpha1.Rollout {
10561056
Canary: &v1alpha1.CanaryStrategy{
10571057
TrafficRouting: &v1alpha1.RolloutTrafficRouting{
10581058
Istio: &v1alpha1.IstioTrafficRouting{
1059-
VirtualService: v1alpha1.IstioVirtualService{
1059+
VirtualService: &v1alpha1.IstioVirtualService{
10601060
Routes: []string{"primary"},
10611061
},
10621062
DestinationRule: &v1alpha1.IstioDestinationRule{

‎utils/istio/istio.go

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -63,22 +63,26 @@ func GetRolloutVirtualServiceKeys(ro *v1alpha1.Rollout) []string {
6363
canary := ro.Spec.Strategy.Canary
6464

6565
if canary == nil || canary.TrafficRouting == nil || canary.TrafficRouting.Istio == nil ||
66-
(canary.TrafficRouting.Istio.VirtualServices == nil && canary.TrafficRouting.Istio.VirtualService.Name == "") ||
67-
(canary.TrafficRouting.Istio.VirtualServices != nil && canary.TrafficRouting.Istio.VirtualService.Name != "") {
66+
(canary.TrafficRouting.Istio.VirtualServices == nil && canary.TrafficRouting.Istio.VirtualService == nil) ||
67+
(canary.TrafficRouting.Istio.VirtualServices != nil && canary.TrafficRouting.Istio.VirtualService != nil) {
6868
return []string{}
6969
}
7070

7171
if MultipleVirtualServiceConfigured(ro) {
7272
virtualServices = ro.Spec.Strategy.Canary.TrafficRouting.Istio.VirtualServices
7373
} else {
74-
virtualServices = []v1alpha1.IstioVirtualService{ro.Spec.Strategy.Canary.TrafficRouting.Istio.VirtualService}
74+
virtualServices = []v1alpha1.IstioVirtualService{*ro.Spec.Strategy.Canary.TrafficRouting.Istio.VirtualService}
7575
}
7676

7777
for _, virtualService := range virtualServices {
7878
namespace, name := GetVirtualServiceNamespaceName(virtualService.Name)
7979
if namespace == "" {
8080
namespace = ro.Namespace
8181
}
82+
if name == "" {
83+
continue
84+
}
85+
8286
virtualServiceKeys = append(virtualServiceKeys, fmt.Sprintf("%s/%s", namespace, name))
8387
}
8488

0 commit comments

Comments
 (0)