diff --git a/build/ci/.test.yml b/build/ci/.test.yml index 225356ea6656130a0737e4b8b4a7b3ee39bcd5b2..3379d59ff0eb2e968ffce59516037e94ca035ef4 100644 --- a/build/ci/.test.yml +++ b/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 diff --git a/nucleus/controller_test.go b/nucleus/controller_test.go index 47fa1026ccc54a35ca3296ce0779fc58af856e9b..1dc5b8bd001270ace098c39a81e027263df7869c 100644 --- a/nucleus/controller_test.go +++ b/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 } diff --git a/nucleus/http.go b/nucleus/http.go index 621d0d30ca7071a675f67d7b8ee62a4bf1340dd9..fe644c7a152dbe52de406d9cd64ccac7abeac493 100644 --- a/nucleus/http.go +++ b/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) diff --git a/nucleus/http_test.go b/nucleus/http_test.go index b7728d9ff87b8359a26623a56fe881ef41b6a3b3..f6b20983a2f18f914bd2a93a63d6e739c01af624 100644 --- a/nucleus/http_test.go +++ b/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(), diff --git a/nucleus/initialise_test.go b/nucleus/initialise_test.go index 932adb147fc0033dbd66557923f27ec965f9c2a4..7fb435337a602f4de461fae08f4b518b6a9f93fe 100644 --- a/nucleus/initialise_test.go +++ b/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 { diff --git a/nucleus/pnd/change.go b/nucleus/pnd/change.go index 8e67b431bfcbd1a2c5e19aafadcac1dbf2503913..553e0368e7cdf2db16746ae8ba278690424cc204 100644 --- a/nucleus/pnd/change.go +++ b/nucleus/pnd/change.go @@ -1,13 +1,35 @@ 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 } diff --git a/nucleus/pnd/change_test.go b/nucleus/pnd/change_test.go new file mode 100644 index 0000000000000000000000000000000000000000..fca3646186d3916566ec5f6dfbdcafa66fa52ae3 --- /dev/null +++ b/nucleus/pnd/change_test.go @@ -0,0 +1,176 @@ +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) + } + }) + } +} diff --git a/nucleus/pnd/initialise_test.go b/nucleus/pnd/initialise_test.go new file mode 100644 index 0000000000000000000000000000000000000000..ff7dcbf9f4d18c0087b6ff826d8cbe3835dae3e6 --- /dev/null +++ b/nucleus/pnd/initialise_test.go @@ -0,0 +1,22 @@ +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 diff --git a/nucleus/principalNetworkDomain_test.go b/nucleus/principalNetworkDomain_test.go index 4b885cdf550139ee2a0410afca2c38d1025202d0..ef4207edc9241670a4ebceb2ba3c56a31c2be3bf 100644 --- a/nucleus/principalNetworkDomain_test.go +++ b/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