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

Ingress e2e test cleanup #577

Merged
merged 3 commits into from
Dec 11, 2018
Merged
Show file tree
Hide file tree
Changes from 2 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
15 changes: 15 additions & 0 deletions cmd/e2e-test/upgrade_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ func TestUpgrade(t *testing.T) {
if err != nil {
t.Fatalf("error waiting for Ingress to stabilize: %v", err)
}

s.PutStatus(e2e.Stable)

if runs == 0 {
Expand Down Expand Up @@ -130,6 +131,20 @@ func TestUpgrade(t *testing.T) {
}

runs++

// If the Master has upgraded and the Ingress is stable,
// we delete the Ingress and exit out of the loop to indicate that
// the test is done.
if s.MasterUpgraded() && !needUpdate {
deleteOptions := &fuzz.GCLBDeleteOptions{
SkipDefaultBackend: true,
}
if err := e2e.WaitForIngressDeletion(context.Background(), gclb, s, ing, deleteOptions); err != nil {
t.Errorf("e2e.WaitForIngressDeletion(..., %q, nil) = %v, want nil", ing.Name, err)
} else {
break
}
}
}
})
}
Expand Down
64 changes: 45 additions & 19 deletions pkg/e2e/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,10 @@ const (
// test can exit.
// 7. The StatusManager loop reads the exit key, then starts shutdown().
type StatusManager struct {
cm *v1.ConfigMap
f *Framework
cm *v1.ConfigMap
f *Framework
informerCh chan struct{}
informersRunning bool
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: informerRunning

}

func NewStatusManager(f *Framework) *StatusManager {
Expand All @@ -94,31 +96,43 @@ func (sm *StatusManager) init() error {
return fmt.Errorf("error creating ConfigMap: %v", err)
}

go func() {
for _ = range time.NewTicker(flushInterval).C {
sm.flush()
}
}()

sm.startInformer()
return nil
}

func (sm *StatusManager) startInformer() {
newIndexer := func() cache.Indexers {
return cache.Indexers{cache.NamespaceIndex: cache.MetaNamespaceIndexFunc}
}
stopCh := make(chan struct{})

sm.informerCh = make(chan struct{})
cmInformer := informerv1.NewConfigMapInformer(sm.f.Clientset, "default", cmPollInterval, newIndexer())
cmInformer.AddEventHandler(cache.ResourceEventHandlerFuncs{
UpdateFunc: func(old, cur interface{}) {
curCm := cur.(*v1.ConfigMap)
if len(curCm.Data[exitKey]) > 0 {
glog.V(2).Infof("ConfigMap was updated with exit switch at %s", curCm.Data[exitKey])
close(stopCh)
close(sm.informerCh)
sm.f.shutdown(0)
}
},
})

go cmInformer.Run(stopCh)

go func() {
for _ = range time.NewTicker(flushInterval).C {
sm.flush()
}
}()
glog.V(4).Info("Started informers")
sm.informersRunning = true
go cmInformer.Run(sm.informerCh)
}

return nil
func (sm *StatusManager) stopInformer() {
glog.V(4).Info("Stopped informers")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you say "Stopped config map informer". Similar for the start func

sm.informersRunning = false
close(sm.informerCh)
}

func (sm *StatusManager) shutdown() {
Expand Down Expand Up @@ -154,13 +168,13 @@ func (sm *StatusManager) flush() {
sm.f.lock.Lock()
defer sm.f.lock.Unlock()

// If master is in the process upgrading, we exit early
if sm.masterUpgrading() {
return
}

// Loop until we successfully update the config map
for {
// Restart ConfigMap informer if it was previously shut down
if sm.masterUpgraded() && !sm.informersRunning {
sm.startInformer()
}

updatedCm, err := sm.f.Clientset.Core().ConfigMaps("default").Get(configMapName, metav1.GetOptions{})
if err != nil {
glog.Warningf("Error getting ConfigMap: %v", err)
Expand All @@ -171,13 +185,25 @@ func (sm *StatusManager) flush() {
}

// K8s considers its version of the ConfigMap to be latest, so we must get
// the configmap from k8s first, then merge in our data.
// the configmap from k8s first.
// We give precedence to the master-upgraded and master-upgrading flags
// set by the external test framework, but otherwise we prioritize
// Ingress statuses set by StatusManager.
for key, value := range sm.cm.Data {
updatedCm.Data[key] = value
if key != masterUpgradedKey && key != masterUpgradingKey {
updatedCm.Data[key] = value
}
}
sm.cm = updatedCm
sm.cm.Name = configMapName

// If master is in the process upgrading, we exit early and turn off the
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be at the top as well.

// ConfigMap informer.
if sm.masterUpgrading() && sm.informersRunning {
sm.stopInformer()
return
}

_, err = sm.f.Clientset.Core().ConfigMaps("default").Update(sm.cm)
if err != nil {
glog.Warningf("Error updating ConfigMap: %v", err)
Expand Down