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.

Sign up for GitHub

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

Emit event for orphaned VirtualServerRoutes #995

Merged
merged 17 commits into from
Jun 25, 2020
Merged

Emit event for orphaned VirtualServerRoutes #995

Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
0726eff
Emit event to orphaned vsrs
LorcanMcVeigh Jun 2, 2020
ff2676f
Removed comment
LorcanMcVeigh Jun 2, 2020
132d9ec
Update vsr when not referenced
LorcanMcVeigh Jun 15, 2020
be8aa9f
Removed comments and unneeeded code
LorcanMcVeigh Jun 16, 2020
c08208a
Add tests
LorcanMcVeigh Jun 17, 2020
2cff0cb
Feedback
LorcanMcVeigh Jun 17, 2020
ffeba6b
Refactor code
LorcanMcVeigh Jun 18, 2020
6df9a09
Feedback
LorcanMcVeigh Jun 18, 2020
fd2b898
Fix tests
LorcanMcVeigh Jun 18, 2020
872ec42
Feedback 2
LorcanMcVeigh Jun 22, 2020
bc09a6a
Refactor test
LorcanMcVeigh Jun 22, 2020
c74f48c
WIP
LorcanMcVeigh Jun 23, 2020
eb6da47
Address feedback
LorcanMcVeigh Jun 23, 2020
66a07e3
feedback 3
LorcanMcVeigh Jun 24, 2020
f5fa9cc
feedback 4
LorcanMcVeigh Jun 24, 2020
20de3dd
apply feedback
LorcanMcVeigh Jun 24, 2020
ba7abbe
feedback 5
LorcanMcVeigh Jun 24, 2020
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
10 changes: 10 additions & 0 deletions internal/configs/configurator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,16 @@ func (cnf *Configurator) addOrUpdateTransportServer(transportServerEx *Transport
return nil
}

// GetVirtualServerRoutesForVirtualServer returns the virtualServerRoutes that a virtualServer
// references, if that virtualServer exists
func (cnf *Configurator) GetVirtualServerRoutesForVirtualServer(key string) []*conf_v1.VirtualServerRoute {
vsFileName := getFileNameForVirtualServerFromKey(key)
if cnf.virtualServers[vsFileName] != nil {
return cnf.virtualServers[vsFileName].VirtualServerRoutes
}
return nil
}

func (cnf *Configurator) updateTLSPassthroughHostsConfig() error {
cfg, duplicatedHosts := generateTLSPassthroughHostsConfig(cnf.tlsPassthroughPairs)

Expand Down
80 changes: 66 additions & 14 deletions internal/k8s/controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -870,20 +870,32 @@ func (lbc *LoadBalancerController) syncVirtualServer(task task) {
lbc.syncQueue.Requeue(task, err)
return
}

previousVSRs := lbc.configurator.GetVirtualServerRoutesForVirtualServer(key)
if !vsExists {
glog.V(2).Infof("Deleting VirtualServer: %v\n", key)

err := lbc.configurator.DeleteVirtualServer(key)
// TO-DO: emit events for referenced VirtualServerRoutes
if err != nil {
glog.Errorf("Error when deleting configuration for %v: %v", key, err)
}
reason := "NoVirtualServerFound"
for _, vsr := range previousVSRs {
msg := fmt.Sprintf("No VirtualServer references VirtualServerRoute %v/%v", vsr.Namespace, vsr.Name)
lbc.recorder.Eventf(vsr, api_v1.EventTypeWarning, reason, msg)

if lbc.reportVsVsrStatusEnabled() {
virtualServersForVSR := []*conf_v1.VirtualServer{}
err = lbc.statusUpdater.UpdateVirtualServerRouteStatusWithReferencedBy(vsr, conf_v1.StateInvalid, reason, msg, virtualServersForVSR)
if err != nil {
glog.Errorf("Error when updating the status for VirtualServerRoute %v/%v: %v", vsr.Namespace, vsr.Name, err)
}
}

}
return
}

glog.V(2).Infof("Adding or Updating VirtualServer: %v\n", key)

vs := obj.(*conf_v1.VirtualServer)

validationErr := validation.ValidateVirtualServer(vs, lbc.isNginxPlus)
Expand All @@ -892,27 +904,33 @@ func (lbc *LoadBalancerController) syncVirtualServer(task task) {
if err != nil {
glog.Errorf("Error when deleting configuration for %v: %v", key, err)
}
reason := "Rejected"
msg := fmt.Sprintf("VirtualServer %v is invalid and was rejected: %v", key, validationErr)

lbc.recorder.Eventf(vs, api_v1.EventTypeWarning, reason, msg)
if lbc.reportVsVsrStatusEnabled() {
LorcanMcVeigh marked this conversation as resolved.
Show resolved Hide resolved
err = lbc.statusUpdater.UpdateVirtualServerStatus(vs, conf_v1.StateInvalid, reason, msg)
lbc.recorder.Eventf(vs, api_v1.EventTypeWarning, "Rejected", "VirtualServer %v is invalid and was rejected: %v", key, validationErr)

if err != nil {
glog.Errorf("Error when updating the status for VirtualServer %v/%v: %v", vs.Namespace, vs.Name, err)
reason := "NoVirtualServerFound"
for _, vsr := range previousVSRs {
msg := fmt.Sprintf("No VirtualServer references VirtualServerRoute %v/%v", vsr.Namespace, vsr.Name)
lbc.recorder.Eventf(vsr, api_v1.EventTypeWarning, reason, msg)

if lbc.reportVsVsrStatusEnabled() {
virtualServersForVSR := []*conf_v1.VirtualServer{}
err = lbc.statusUpdater.UpdateVirtualServerRouteStatusWithReferencedBy(vsr, conf_v1.StateInvalid, reason, msg, virtualServersForVSR)
if err != nil {
glog.Errorf("Error when updating the status for VirtualServerRoute %v/%v: %v", vsr.Namespace, vsr.Name, err)
}
}
}

// TO-DO: emit events for referenced VirtualServerRoutes
return
}

var handledVSRs []*conf_v1.VirtualServerRoute

vsEx, vsrErrors := lbc.createVirtualServer(vs)

for _, vsrError := range vsrErrors {
lbc.recorder.Eventf(vs, api_v1.EventTypeWarning, "IgnoredVirtualServerRoute", "Ignored VirtualServerRoute %v: %v", vsrError.VirtualServerRouteNsName, vsrError.Error)
if vsrError.VirtualServerRoute != nil {
handledVSRs = append(handledVSRs, vsrError.VirtualServerRoute)
lbc.recorder.Eventf(vsrError.VirtualServerRoute, api_v1.EventTypeWarning, "Ignored", "Ignored by VirtualServer %v/%v: %v", vs.Namespace, vs.Name, vsrError.Error)
}
}
Expand Down Expand Up @@ -969,15 +987,49 @@ func (lbc *LoadBalancerController) syncVirtualServer(task task) {
lbc.recorder.Eventf(vsr, vsrEventType, vsrEventTitle, msg)

if lbc.reportVsVsrStatusEnabled() {
virtualServersForVSR := findVirtualServersForVirtualServerRoute(lbc.getVirtualServers(), vsr)
err = lbc.statusUpdater.UpdateVirtualServerRouteStatusWithReferencedBy(vsr, state, vsrEventTitle, msg, virtualServersForVSR)
vss := []*conf_v1.VirtualServer{vs}
err = lbc.statusUpdater.UpdateVirtualServerRouteStatusWithReferencedBy(vsr, state, vsrEventTitle, msg, vss)

if err != nil {
glog.Errorf("Error when updating the status for VirtualServerRoute %v/%v: %v", vsr.Namespace, vsr.Name, err)
}
}

handledVSRs = append(handledVSRs, vsr)
}

orphanedVSRs := filterOutVirtualServerRoutes(previousVSRs, handledVSRs)
reason := "NoVirtualServerFound"
for _, vsr := range orphanedVSRs {
msg := fmt.Sprintf("No VirtualServer references VirtualServerRoute %v/%v", vsr.Namespace, vsr.Name)
lbc.recorder.Eventf(vsr, api_v1.EventTypeWarning, "Ignored", msg)
LorcanMcVeigh marked this conversation as resolved.
Show resolved Hide resolved
if lbc.reportVsVsrStatusEnabled() {
var emptyVSS []*conf_v1.VirtualServer
LorcanMcVeigh marked this conversation as resolved.
Show resolved Hide resolved
err := lbc.statusUpdater.UpdateVirtualServerRouteStatusWithReferencedBy(vsr, conf_v1.StateInvalid, reason, msg, emptyVSS)
if err != nil {
glog.Errorf("Error when updating the status for VirtualServerRoute %v/%v: %v", vsr.Namespace, vsr.Name, err)
}
}
}
}

func filterOutVirtualServerRoutes(previousVSRs []*conf_v1.VirtualServerRoute, handledVSRs []*conf_v1.VirtualServerRoute) []*conf_v1.VirtualServerRoute {
LorcanMcVeigh marked this conversation as resolved.
Show resolved Hide resolved
var orphanedVSRs []*conf_v1.VirtualServerRoute
for _, r := range previousVSRs {
LorcanMcVeigh marked this conversation as resolved.
Show resolved Hide resolved
isIn := false
key := fmt.Sprintf("%s/%s", r.Namespace, r.Name)
for _, r1 := range handledVSRs {
vsrKey := fmt.Sprintf("%s/%s", r1.Namespace, r1.Name)
if key == vsrKey {
isIn = true
break
}
}
if !isIn {
orphanedVSRs = append(orphanedVSRs, r)
}
}
return orphanedVSRs
}

func (lbc *LoadBalancerController) syncVirtualServerRoute(task task) {
Expand Down
50 changes: 50 additions & 0 deletions internal/k8s/controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1382,6 +1382,56 @@ func TestFindVirtualServerRoutesForService(t *testing.T) {
}
}

func TestCheckForVirtualServerRoute(t *testing.T) {
LorcanMcVeigh marked this conversation as resolved.
Show resolved Hide resolved
vsr1 := conf_v1.VirtualServerRoute{
ObjectMeta: meta_v1.ObjectMeta{
Name: "vsr-1",
Namespace: "ns-1",
},
}

vsr2 := conf_v1.VirtualServerRoute{
ObjectMeta: meta_v1.ObjectMeta{
Name: "vsr-2",
Namespace: "ns-1",
},
}

vsr3 := conf_v1.VirtualServerRoute{
ObjectMeta: meta_v1.ObjectMeta{
Name: "vsr-3",
Namespace: "ns-2",
},
}

vsr4 := conf_v1.VirtualServerRoute{
ObjectMeta: meta_v1.ObjectMeta{
Name: "vsr-4",
Namespace: "ns-1",
},
}

vsr5 := conf_v1.VirtualServerRoute{
ObjectMeta: meta_v1.ObjectMeta{
Name: "vsr-5",
Namespace: "ns-3",
},
}

vsrs := []*conf_v1.VirtualServerRoute{&vsr1, &vsr2, &vsr3, &vsr4, &vsr5}

handledVSRs := []*conf_v1.VirtualServerRoute{&vsr3}

expected := []*conf_v1.VirtualServerRoute{&vsr1, &vsr2, &vsr4, &vsr5}

result := filterOutVirtualServerRoutes(vsrs, handledVSRs)

if !reflect.DeepEqual(result, expected) {
t.Errorf("checkForVirtualServerRoute return %v but expected %v", result, expected)
LorcanMcVeigh marked this conversation as resolved.
Show resolved Hide resolved
}

LorcanMcVeigh marked this conversation as resolved.
Show resolved Hide resolved
}

func TestFindTransportServersForService(t *testing.T) {
ts1 := conf_v1alpha1.TransportServer{
ObjectMeta: meta_v1.ObjectMeta{
Expand Down