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

AppInterface enhancements for subscription #67

Merged
merged 7 commits into from
Apr 25, 2023
Merged

AppInterface enhancements for subscription #67

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
40c8ace
AppInterface enhancements for subscription
sachinholla Mar 31, 2023
6bc3fde
Merge branch 'master' into subscribe_app_intf
sachinholla Apr 1, 2023
3c336d8
Update error types, test case styles, nil checks, etc
sachinholla Apr 3, 2023
f5c8c92
Remove tlerr import alias in yanglib_app
sachinholla Apr 20, 2023
97b8b56
Non-pointer params in transleSubscribe and processSubscribe
sachinholla Apr 20, 2023
d4669bc
Reorg old & new version comprision logic in EntryCompare func
sachinholla Apr 20, 2023
808112f
appInterface method cleanup in acl_app
sachinholla Apr 20, 2023
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
112 changes: 18 additions & 94 deletions translib/acl_app.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,11 @@ package translib

import (
"bytes"
"errors"
"fmt"
"reflect"
"strconv"
"strings"

"github.com/Azure/sonic-mgmt-common/translib/db"
"github.com/Azure/sonic-mgmt-common/translib/ocbinds"
"github.com/Azure/sonic-mgmt-common/translib/tlerr"
Expand Down Expand Up @@ -127,109 +127,27 @@ func (app *AclApp) getAppRootObject() *ocbinds.OpenconfigAcl_Acl {
}

func (app *AclApp) translateCreate(d *db.DB) ([]db.WatchKeys, error) {
var err error
var keys []db.WatchKeys
log.Info("translateCreate:acl:path =", app.pathInfo.Template)

keys, err = app.translateCRUCommon(d, CREATE)
return keys, err
return app.translateCRUCommon(d, CREATE)
}

func (app *AclApp) translateUpdate(d *db.DB) ([]db.WatchKeys, error) {
var err error
var keys []db.WatchKeys
log.Info("translateUpdate:acl:path =", app.pathInfo.Template)

keys, err = app.translateCRUCommon(d, UPDATE)
return keys, err
return app.translateCRUCommon(d, UPDATE)
}

func (app *AclApp) translateReplace(d *db.DB) ([]db.WatchKeys, error) {
var err error
var keys []db.WatchKeys
log.Info("translateReplace:acl:path =", app.pathInfo.Template)

keys, err = app.translateCRUCommon(d, REPLACE)
return keys, err
return app.translateCRUCommon(d, REPLACE)
}

func (app *AclApp) translateDelete(d *db.DB) ([]db.WatchKeys, error) {
var err error
var keys []db.WatchKeys
log.Info("translateDelete:acl:path =", app.pathInfo.Template)

return keys, err
return nil, nil
}

func (app *AclApp) translateGet(dbs [db.MaxDB]*db.DB) error {
var err error
log.Info("translateGet:acl:path =", app.pathInfo.Template)
return err
return nil
}

func (app *AclApp) translateAction(dbs [db.MaxDB]*db.DB) error {
err := errors.New("Not supported")
return err
}

func (app *AclApp) translateSubscribe(dbs [db.MaxDB]*db.DB, path string) (*notificationOpts, *notificationInfo, error) {
pathInfo := NewPathInfo(path)
notifInfo := notificationInfo{dbno: db.ConfigDB}
notSupported := tlerr.NotSupportedError{
Format: "Subscribe not supported", Path: path}

if isSubtreeRequest(pathInfo.Template, "/openconfig-acl:acl/acl-sets") {
// Subscribing to top level ACL record is not supported. It requires listening
// to 2 tables (ACL and ACL_RULE); TransLib does not support it yet
if pathInfo.HasSuffix("/acl-sets") ||
pathInfo.HasSuffix("/acl-set") ||
pathInfo.HasSuffix("/acl-set{}{}") {
log.Errorf("Subscribe not supported for top level ACL %s", pathInfo.Template)
return nil, nil, notSupported
}

t, err := getAclTypeOCEnumFromName(pathInfo.Var("type"))
if err != nil {
return nil, nil, err
}

aclkey := getAclKeyStrFromOCKey(pathInfo.Var("name"), t)

if strings.Contains(pathInfo.Template, "/acl-entry{}") {
// Subscribe for one rule
rulekey := "RULE_" + pathInfo.Var("sequence-id")
notifInfo.table = db.TableSpec{Name: RULE_TABLE}
notifInfo.key = asKey(aclkey, rulekey)
notifInfo.needCache = !pathInfo.HasSuffix("/acl-entry{}")

} else if pathInfo.HasSuffix("/acl-entries") || pathInfo.HasSuffix("/acl-entry") {
// Subscribe for all rules of an ACL
notifInfo.table = db.TableSpec{Name: RULE_TABLE}
notifInfo.key = asKey(aclkey, "*")

} else {
// Subscibe for ACL fields only
notifInfo.table = db.TableSpec{Name: ACL_TABLE}
notifInfo.key = asKey(aclkey)
notifInfo.needCache = true
}

} else if isSubtreeRequest(pathInfo.Template, "/openconfig-acl:acl/interfaces") {
// Right now interface binding config is maintained within ACL
// table itself. Multiple ACLs can be bound to one intf; one
// inname can occur in multiple ACL entries. So we cannot map
// interface binding xpaths to specific ACL table entry keys.
// For now subscribe for full ACL table!!
notifInfo.table = db.TableSpec{Name: ACL_TABLE}
notifInfo.key = asKey("*")
notifInfo.needCache = true

} else {
log.Errorf("Unknown path %s", pathInfo.Template)
return nil, nil, notSupported
}

return nil, &notifInfo, nil
return tlerr.NotSupported("unsupported")
}

func (app *AclApp) processCreate(d *db.DB) (SetResponse, error) {
Expand Down Expand Up @@ -295,10 +213,7 @@ func (app *AclApp) processGet(dbs [db.MaxDB]*db.DB) (GetResponse, error) {
}

func (app *AclApp) processAction(dbs [db.MaxDB]*db.DB) (ActionResponse, error) {
var resp ActionResponse
err := errors.New("Not implemented")

return resp, err
return ActionResponse{}, tlerr.New("not implemented")
}

func (app *AclApp) translateCRUCommon(d *db.DB, opcode int) ([]db.WatchKeys, error) {
Expand Down Expand Up @@ -1716,10 +1631,19 @@ func getAclKeyStrFromOCKey(aclname string, acltype ocbinds.E_OpenconfigAcl_ACL_T
return aclN + "_" + aclT
}

/* Check if targetUriPath is child (subtree) of nodePath
/*
Check if targetUriPath is child (subtree) of nodePath
The return value can be used to decide if subtrees needs
to visited to fill the data or not.
*/
func isSubtreeRequest(targetUriPath string, nodePath string) bool {
return strings.HasPrefix(targetUriPath, nodePath)
}

func (app *AclApp) translateSubscribe(req translateSubRequest) (translateSubResponse, error) {
return emptySubscribeResponse(req.path)
}

func (app *AclApp) processSubscribe(req processSubRequest) (processSubResponse, error) {
return processSubResponse{}, tlerr.New("not implemented")
}
9 changes: 7 additions & 2 deletions translib/api_tests_app.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
// //
////////////////////////////////////////////////////////////////////////////////

//go:build test
// +build test

package translib
Expand Down Expand Up @@ -101,8 +102,12 @@ func (app *apiTests) translateAction(dbs [db.MaxDB]*db.DB) error {
return nil
}

func (app *apiTests) translateSubscribe(dbs [db.MaxDB]*db.DB, path string) (*notificationOpts, *notificationInfo, error) {
return nil, nil, nil
func (app *apiTests) translateSubscribe(req translateSubRequest) (translateSubResponse, error) {
return emptySubscribeResponse(req.path)
}

func (app *apiTests) processSubscribe(req processSubRequest) (processSubResponse, error) {
return processSubResponse{}, tlerr.New("not implemented")
}

func (app *apiTests) processCreate(d *db.DB) (SetResponse, error) {
Expand Down
48 changes: 25 additions & 23 deletions translib/app_interface.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
// //
// Unless required by applicable law or agreed to in writing, software //
// distributed under the License is distributed on an "AS IS" BASIS, //
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. //
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. //
// See the License for the specific language governing permissions and //
// limitations under the License. //
// //
Expand All @@ -31,22 +31,23 @@ package translib

import (
"errors"
log "github.com/golang/glog"
"github.com/openconfig/ygot/ygot"
"reflect"
"strings"

"github.com/Azure/sonic-mgmt-common/translib/db"
log "github.com/golang/glog"
"github.com/openconfig/ygot/ygot"
)

//Structure containing app module information
// Structure containing app module information
type appInfo struct {
appType reflect.Type
ygotRootType reflect.Type
isNative bool
tablesToWatch []*db.TableSpec
}

//Structure containing the app data coming from translib infra
// Structure containing the app data coming from translib infra
type appData struct {
path string
payload []byte
Expand All @@ -59,23 +60,23 @@ type appData struct {
// These include RESTCONF query parameters like - depth, fields etc.
type appOptions struct {

// depth limits subtree levels in the response data.
// 0 indicates unlimited depth.
// Valid for GET API only.
depth uint
// depth limits subtree levels in the response data.
// 0 indicates unlimited depth.
// Valid for GET API only.
depth uint

// deleteEmptyEntry indicates if the db entry should be deleted upon
// deletion of last field. This is a non standard option.
deleteEmptyEntry bool
// deleteEmptyEntry indicates if the db entry should be deleted upon
// deletion of last field. This is a non standard option.
deleteEmptyEntry bool
}

//map containing the base path to app module info
// map containing the base path to app module info
var appMap map[string]*appInfo

//array containing all the supported models
// array containing all the supported models
var models []ModelData

//Interface for all App Modules
// Interface for all App Modules
type appInterface interface {
initialize(data appData)
translateCreate(d *db.DB) ([]db.WatchKeys, error)
Expand All @@ -84,16 +85,17 @@ type appInterface interface {
translateDelete(d *db.DB) ([]db.WatchKeys, error)
translateGet(dbs [db.MaxDB]*db.DB) error
translateAction(dbs [db.MaxDB]*db.DB) error
translateSubscribe(dbs [db.MaxDB]*db.DB, path string) (*notificationOpts, *notificationInfo, error)
translateSubscribe(req translateSubRequest) (translateSubResponse, error)
processCreate(d *db.DB) (SetResponse, error)
processUpdate(d *db.DB) (SetResponse, error)
processReplace(d *db.DB) (SetResponse, error)
processDelete(d *db.DB) (SetResponse, error)
processGet(dbs [db.MaxDB]*db.DB) (GetResponse, error)
processAction(dbs [db.MaxDB]*db.DB) (ActionResponse, error)
processSubscribe(req processSubRequest) (processSubResponse, error)
}

//App modules will use this function to register with App interface during boot up
// App modules will use this function to register with App interface during boot up
func register(path string, info *appInfo) error {
var err error
log.Info("Registering for path =", path)
Copy link

Choose a reason for hiding this comment

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

While correcting this file, it would be helpful to convert stuff like this to a more explicit call like log.Infof("Registering for path =%s", path)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are you suggesting to use Infof() instead of Info() ??
I am not performing any such cleanup of the existing code in this PR. I am only introducing translateSubscribe() & peocessSubscribe() methods and types used by them. All other changes you see (like line #98 here) are due to gofmt.

Expand All @@ -114,7 +116,7 @@ func register(path string, info *appInfo) error {
return err
}

//Adds the model information to the supported models array
// Adds the model information to the supported models array
func addModel(model *ModelData) error {
var err error

Expand All @@ -124,7 +126,7 @@ func addModel(model *ModelData) error {
return err
}

//Translib infra will use this function get the app info for a given path
// Translib infra will use this function get the app info for a given path
func getAppModuleInfo(path string) (*appInfo, error) {
log.Info("getAppModule called for path =", path)

Expand All @@ -135,23 +137,23 @@ func getAppModuleInfo(path string) (*appInfo, error) {

log.Info("found the entry in the map for path =", pattern)

return app, nil
return app, nil
}

/* If no specific app registered fallback to default/common app */
log.Infof("No app module registered for path %s hence fallback to default/common app", path)
app := appMap["*"]

return app, nil
return app, nil
}

//Get all the supported models
// Get all the supported models
func getModels() []ModelData {

return models
}

//Creates a new app from the appType and returns it as an appInterface
// Creates a new app from the appType and returns it as an appInterface
func getAppInterface(appType reflect.Type) (appInterface, error) {
var err error
appInstance := reflect.New(appType)
Expand Down
57 changes: 5 additions & 52 deletions translib/common_app.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,59 +129,12 @@ func (app *CommonApp) translateGet(dbs [db.MaxDB]*db.DB) error {
return err
}

func (app *CommonApp) translateSubscribe(dbs [db.MaxDB]*db.DB, path string) (*notificationOpts, *notificationInfo, error) {
var err error
var subscDt transformer.XfmrTranslateSubscribeInfo
var notifInfo notificationInfo
var notifOpts notificationOpts
txCache := new(sync.Map)
err = tlerr.NotSupportedError{Format: "Subscribe not supported", Path: path}

log.Info("tranlateSubscribe:path", path)
subscDt, err = transformer.XlateTranslateSubscribe(path, dbs, txCache)
if subscDt.PType == transformer.OnChange {
notifOpts.pType = OnChange
} else {
notifOpts.pType = Sample
}
notifOpts.mInterval = subscDt.MinInterval
notifOpts.isOnChangeSupported = subscDt.OnChange
if err != nil {
log.Infof("returning: notificationOpts - %v, nil, error - %v", notifOpts, err)
return &notifOpts, nil, err
}
if subscDt.DbDataMap == nil {
log.Infof("DB data is nil so returning: notificationOpts - %v, nil, error - %v", notifOpts, err)
return &notifOpts, nil, err
} else {
for dbNo, dbDt := range(subscDt.DbDataMap) {
if (len(dbDt) == 0) { //ideally all tables for a given uri should be from same DB
continue
}
log.Infof("Adding to notifInfo, Db Data - %v for DB No - %v", dbDt, dbNo)
notifInfo.dbno = dbNo
// in future there will be, multi-table in a DB, support from translib, for now its just single table
for tblNm, tblDt := range(dbDt) {
notifInfo.table = db.TableSpec{Name:tblNm}
if (len(tblDt) == 1) {
for tblKy := range(tblDt) {
notifInfo.key = asKey(tblKy)
notifInfo.needCache = subscDt.NeedCache
}
} else {
if (len(tblDt) > 1) {
log.Warningf("More than one DB key found for subscription path - %v", path)
} else {
log.Warningf("No DB key found for subscription path - %v", path)
}
return &notifOpts, nil, err
}
func (app *CommonApp) translateSubscribe(req translateSubRequest) (translateSubResponse, error) {
return emptySubscribeResponse(req.path)
}

}
}
}
log.Infof("For path - %v, returning: notifOpts - %v, notifInfo - %v, error - nil", path, notifOpts, notifInfo)
return &notifOpts, &notifInfo, nil
func (app *CommonApp) processSubscribe(req processSubRequest) (processSubResponse, error) {
return processSubResponse{}, tlerr.New("not implemented")
}

func (app *CommonApp) translateAction(dbs [db.MaxDB]*db.DB) error {
Expand Down
Loading