basic tests for change

build/ci/.test.yml

@@ -41,8 +41,8 @@ unit-test:
     - go test -short -race $(go list ./... | grep -v /forks/ | grep -v /api/ | grep -v /mocks ) -v -coverprofile=coverage.out
   <<: *test
 
-http-api-test:
+controller-test:
   script:
     - cd ./nucleus
-    - go test -race -v -run Test_httpApi -coverprofile=coverage.out
+    - go test -race -v -run TestRun -coverprofile=../coverage.out
   <<: *test
\ No newline at end of file

nucleus/controller_test.go

@@ -9,6 +9,9 @@ import (
 )
 
 func TestRun(t *testing.T) {
+	if testing.Short() {
+		t.Skip("this test is executed separately")
+	}
 	type args struct {
 		request string
 	}

nucleus/http.go

@@ -199,11 +199,9 @@ func httpHandler(writer http.ResponseWriter, request *http.Request) {
 	case "change-list":
 		changes := pnd.ListCommitted()
 		writeIDs(writer, "Tentative changes", changes)
-		writer.WriteHeader(http.StatusOK)
 	case "change-list-pending":
 		changes := pnd.ListPending()
 		writeIDs(writer, "Pending changes", changes)
-		writer.WriteHeader(http.StatusOK)
 	case "change-commit":
 		cuid, err := uuid.Parse(query.Get("cuid"))
 		if err != nil {
@@ -215,7 +213,11 @@ func httpHandler(writer http.ResponseWriter, request *http.Request) {
 			handleServerError(writer, err)
 			return
 		}
-		change.(*Change).Commit()
+		err = change.(*Change).Commit()
+		if err != nil {
+			handleServerError(writer, err)
+			return
+		}
 		writer.WriteHeader(http.StatusAccepted)
 	case "change-confirm":
 		cuid, err := uuid.Parse(query.Get("cuid"))
@@ -228,7 +230,11 @@ func httpHandler(writer http.ResponseWriter, request *http.Request) {
 			handleServerError(writer, err)
 			return
 		}
-		change.(*Change).Confirm()
+		err = change.(*Change).Confirm()
+		if err != nil {
+			handleServerError(writer, err)
+			return
+		}
 		writer.WriteHeader(http.StatusAccepted)
 	default:
 		writer.WriteHeader(http.StatusBadRequest)

nucleus/http_test.go

@@ -39,9 +39,6 @@ func testSetupHTTP() {
 }
 
 func Test_httpApi(t *testing.T) {
-	if testing.Short() {
-		t.Skip("this test is executed separately")
-	}
 	tests := []struct {
 		name    string
 		request string
@@ -139,6 +136,36 @@ func Test_httpApi(t *testing.T) {
 			want:    &http.Response{StatusCode: http.StatusOK},
 			wantErr: false,
 		},
+		{
+			name:    "change list",
+			request: apiEndpoint + "/api?q=change-list" + args + "&path=/system/config/hostname",
+			want:    &http.Response{StatusCode: http.StatusOK},
+			wantErr: false,
+		},
+		{
+			name:    "change list pending",
+			request: apiEndpoint + "/api?q=change-list-pending" + args + "&path=/system/config/hostname",
+			want:    &http.Response{StatusCode: http.StatusOK},
+			wantErr: false,
+		},
+		{
+			name:    "change commit",
+			request: apiEndpoint + "/api?q=change-commit" + args + "&cuid=" + uuid.New().String(),
+			want:    &http.Response{StatusCode: http.StatusOK},
+			wantErr: false,
+		},
+		{
+			name:    "change confirm",
+			request: apiEndpoint + "/api?q=change-confirm" + args + "&cuid="  + uuid.New().String(),
+			want:    &http.Response{StatusCode: http.StatusOK},
+			wantErr: false,
+		},
+		{
+			name:    "bad request",
+			request: apiEndpoint + "/api?q=bad-request",
+			want:    &http.Response{StatusCode: http.StatusBadRequest},
+			wantErr: false,
+		},
 		{
 			name:    "internal server errror: wrong pnd",
 			request: apiEndpoint + "/api?pnd=" + uuid.New().String(),

nucleus/initialise_test.go

@@ -108,29 +108,10 @@ func newGnmiTransportOptions() *GnmiTransportOptions {
 func readTestUUIDs() {
 	var err error
 	did, err = uuid.Parse("4d8246f8-e884-41d6-87f5-c2c784df9e44")
-	if err != nil {
-		log.Fatal(err)
-	}
-
 	mdid, err = uuid.Parse("688a264e-5f85-40f8-bd13-afc42fcd5c7a")
-	if err != nil {
-		log.Fatal(err)
-	}
-
 	defaultSbiID, err = uuid.Parse("b70c8425-68c7-4d4b-bb5e-5586572bd64b")
-	if err != nil {
-		log.Fatal(err)
-	}
-
 	defaultPndID, err = uuid.Parse("b4016412-eec5-45a1-aa29-f59915357bad")
-	if err != nil {
-		log.Fatal(err)
-	}
-
 	ocUUID, err = uuid.Parse("5e252b70-38f2-4c99-a0bf-1b16af4d7e67")
-	if err != nil {
-		log.Fatal(err)
-	}
 	iid, err = uuid.Parse("8495a8ac-a1e8-418e-b787-10f5878b2690")
 	altIid, err = uuid.Parse("edc5de93-2d15-4586-b2a7-fb1bc770986b")
 	if err != nil {

nucleus/pnd/change.go

 package pnd
 
 import (
+	"errors"
 	"github.com/google/uuid"
 	"github.com/openconfig/ygot/ygot"
 	log "github.com/sirupsen/logrus"
+	"golang.org/x/net/context"
+	"os"
 	"sync"
 	"time"
 )
 
+var changeTimeout time.Duration
+
+func init() {
+	timeout, err := time.ParseDuration(os.Getenv("GOSDN_CHANGE_TIMEOUT"))
+	if err != nil {
+		log.Fatal(err)
+	}
+	if timeout != time.Duration(0) {
+		changeTimeout = timeout
+	} else {
+		var err error
+		changeTimeout, err = time.ParseDuration("10m")
+		if err != nil {
+			log.Fatal()
+		}
+	}
+	log.Debugf("change timeout set to %v", changeTimeout)
+}
+
 func NewChange(device uuid.UUID, currentState ygot.GoStruct, change ygot.GoStruct, callback func(ygot.GoStruct, ygot.GoStruct) error) *Change {
 	return &Change{
 		cuid:          uuid.New(),
@@ -21,7 +43,7 @@ func NewChange(device uuid.UUID, currentState ygot.GoStruct, change ygot.GoStruc
 	}
 }
 
-// Change is an intended change to a OND. It is unique and immutable.
+// Change is an intended change to an OND. It is unique and immutable.
 // It has a cuid, a timestamp, and holds both the previous and the new
 // state. It keeps track if the state is committed and confirmed. A callback
 // exists to acess the proper transport for the changed OND
@@ -35,53 +57,66 @@ type Change struct {
 	confirmed     bool
 	callback      func(ygot.GoStruct, ygot.GoStruct) error
 	lock          sync.RWMutex
+	cancelFunc    context.CancelFunc
 }
 
 func (c *Change) ID() uuid.UUID {
 	return c.cuid
 }
 
-func (c *Change) Commit() {
+func (c *Change) Commit() error {
 	c.committed = true
 	if err := c.callback(c.intendedState, c.previousState); err != nil {
-		log.WithFields(log.Fields{
-			"change uuid": c.cuid,
-			"device uuid": c.duid,
-		}).Error(err)
-		log.WithFields(log.Fields{
-			"change uuid": c.cuid,
-			"device uuid": c.duid,
-		}).Debug("change commited")
+		return err
 	}
-	go func() {
-		time.Sleep(time.Minute * 10)
+	log.WithFields(log.Fields{
+		"change uuid": c.cuid,
+		"device uuid": c.duid,
+	}).Debug("change commited")
+	ctx, cancel := context.WithCancel(context.Background())
+	c.cancelFunc = cancel
+	go c.rollbackHandler(ctx)
+	return nil
+}
+
+func (c *Change) rollbackHandler(ctx context.Context) {
+	select {
+	case <-ctx.Done():
+		return
+	case <-time.Tick(changeTimeout):
 		c.lock.RLock()
 		defer c.lock.RUnlock()
 		if !c.confirmed {
-			c.Rollback()
+			err := c.callback(c.previousState, c.intendedState)
+			if err != nil {
+				log.WithFields(log.Fields{
+					"change uuid": c.cuid,
+					"device uuid": c.duid,
+					"error":       err,
+				}).Error("rollback error")
+			}
 			log.WithFields(log.Fields{
 				"change uuid": c.cuid,
 				"device uuid": c.duid,
-			}).Error("change timed out")
+			}).Info("change timed out")
 		}
-	}()
-}
-
-func (c *Change) Rollback() {
-	if err := c.callback(c.previousState, c.intendedState); err != nil {
-		log.WithFields(log.Fields{
-			"change uuid": c.cuid,
-			"device uuid": c.duid,
-		}).Error(err)
 	}
 }
 
-func (c *Change) Confirm() {
+func (c *Change) Confirm() error {
+	c.lock.RLock()
+	if !c.committed {
+		defer c.lock.RUnlock()
+		return errors.New("cannot confirm uncommitted change")
+	}
+	c.lock.RUnlock()
 	c.lock.Lock()
 	defer c.lock.Unlock()
 	c.confirmed = true
+	c.cancelFunc()
 	log.WithFields(log.Fields{
 		"change uuid": c.cuid,
 		"device uuid": c.duid,
 	}).Info("change confirmed")
+	return nil
 }

nucleus/pnd/change_test.go

+package pnd
+
+import (
+	"context"
+	"github.com/google/uuid"
+	"github.com/openconfig/ygot/exampleoc"
+	"github.com/openconfig/ygot/ygot"
+	"os"
+	"reflect"
+	"sync"
+	"testing"
+	"time"
+)
+
+var commit = "commit"
+var rollback = "rollback"
+
+var commitDevice = &exampleoc.Device{
+	System: &exampleoc.System{
+		Hostname: &commit,
+	},
+}
+
+var rollbackDevice = &exampleoc.Device{
+	System: &exampleoc.System{
+		Hostname: &rollback,
+	},
+}
+
+func TestChange_Commit(t *testing.T) {
+	callback := make(chan string)
+	tests := []struct {
+		name    string
+		want    string
+		wantErr bool
+	}{
+		{
+			name: commit,
+			want:    commit,
+			wantErr: false,
+		},
+		{
+			name: rollback,
+			want:    rollback,
+			wantErr: false,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			c := &Change{
+				cuid:          changeUUID,
+				duid:          did,
+				timestamp:     time.Now(),
+				previousState: rollbackDevice,
+				intendedState: commitDevice,
+				callback:      func(first ygot.GoStruct, second ygot.GoStruct) error {
+					hostname := *first.(*exampleoc.Device).System.Hostname
+					t.Logf("hostname: %v", hostname)
+					callback <- hostname
+					return nil
+				},
+				lock:          sync.RWMutex{},
+			}
+			go func() {
+				time.Sleep(time.Millisecond * 10)
+				if err := c.Commit(); (err != nil) != tt.wantErr {
+					t.Errorf("Commit() error = %v, wantErr %v", err, tt.wantErr)
+				}
+				if tt.name == "rollback" {
+					timeout, err := time.ParseDuration(os.Getenv("GOSDN_CHANGE_TIMEOUT"))
+					if err != nil {
+						t.Error(err)
+					}
+					time.Sleep(timeout)
+				}
+			}()
+			var got string
+			switch tt.name {
+			case commit:
+				got = <-callback
+			case rollback:
+				_ = <-callback
+				got = <-callback
+			}
+			if !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("Commit() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+	close(callback)
+}
+
+func TestChange_Confirm(t *testing.T) {
+	_, cancel := context.WithCancel(context.Background())
+	type fields struct {
+		cuid          uuid.UUID
+		duid          uuid.UUID
+		timestamp     time.Time
+		previousState ygot.GoStruct
+		intendedState ygot.GoStruct
+		callback      func(ygot.GoStruct, ygot.GoStruct) error
+		committed     bool
+	}
+	tests := []struct {
+		name   string
+		fields fields
+		wantErr bool
+	}{
+		{
+			name: "committed",
+			fields: fields{
+				committed: true,
+			},
+			wantErr: false,
+		},
+		{
+			name:   "uncommitted",
+			fields: fields{},
+			wantErr: true,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			c := &Change{
+				cuid:      tt.fields.cuid,
+				duid:      tt.fields.duid,
+				timestamp: tt.fields.timestamp,
+				previousState: &exampleoc.Device{
+					System: &exampleoc.System{
+						Hostname: &rollback,
+					},
+				},
+				intendedState: &exampleoc.Device{
+					System: &exampleoc.System{
+						Hostname: &commit,
+					},
+				},
+				callback:   tt.fields.callback,
+				committed:  tt.fields.committed,
+				cancelFunc: cancel,
+				lock:       sync.RWMutex{},
+			}
+			if err := c.Confirm(); (err != nil) != tt.wantErr {
+				t.Errorf("Confirm() error = %v, wantErr %v", err, tt.wantErr)
+			}
+		})
+	}
+	cancel()
+}
+
+func TestChange_ID(t *testing.T) {
+	type fields struct {
+		cuid uuid.UUID
+	}
+	tests := []struct {
+		name   string
+		fields fields
+		want   uuid.UUID
+	}{
+		{
+			name:   "default",
+			fields: fields{cuid: changeUUID},
+			want:   changeUUID,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			c := &Change{
+				cuid: tt.fields.cuid,
+			}
+			if got := c.ID(); !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("ID() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}

nucleus/pnd/initialise_test.go

+package pnd
+
+import (
+	"github.com/google/uuid"
+	log "github.com/sirupsen/logrus"
+	"os"
+	"testing"
+)
+
+// UUIDs for test cases
+var changeUUID uuid.UUID
+var did uuid.UUID
+
+func TestMain(m *testing.M) {
+	var err error
+	changeUUID, err = uuid.Parse("cfbb96cd-ecad-45d1-bebf-1851760f5087")
+	did, err = uuid.Parse("4d8246f8-e884-41d6-87f5-c2c784df9e44")
+	if err != nil {
+		log.Fatal(err)
+	}
+	os.Exit(m.Run())
+}
\ No newline at end of file

nucleus/principalNetworkDomain_test.go

@@ -524,6 +524,7 @@ func Test_pndImplementation_RequestAll(t *testing.T) {
 }
 
 func Test_pndImplementation_ChangeOND(t *testing.T) {
+	t.Fail()
 	type fields struct {
 		name             string
 		description      string