Skip to content

Refactor

Ondrej Fabry edited this page Apr 17, 2020 · 22 revisions

This documents contains info related to refactor of cn-infra API.


April 2020

Motivation

Main Problems

  • package config is hard to use and is missing support for single config file
  • API for datasync and db/keyval are very confusing
  • lookup of plugin dependencies is black magic and not very flexible

Other Issues

  • term plugin is totally overused and does not really represent its use
  • exec essentially contains two different implementation for process management
  • depends on several unnecessary Go modules (uuid, render..)
  • package agent has unfitting name and mixed responsibilities
  • some packages with common generic functionality are part of vpp-agent repo

Analysis

Usage

Here is usage summary for cn-infra packages.

vpp-agent

imports 31, depends on 45
# vpp-agent imports following cn-infra packages (excluding examples/tests)
  1 go.ligato.io/cn-infra/v2/datasync/msgsync
  1 go.ligato.io/cn-infra/v2/db/keyval/consul
  1 go.ligato.io/cn-infra/v2/db/keyval/kvproto
  1 go.ligato.io/cn-infra/v2/db/keyval/redis
  1 go.ligato.io/cn-infra/v2/exec/supervisor
  1 go.ligato.io/cn-infra/v2/health/statuscheck/model/status
  1 go.ligato.io/cn-infra/v2/logging/logmanager
  1 go.ligato.io/cn-infra/v2/messaging/kafka
  1 go.ligato.io/cn-infra/v2/rpc/rest/security/model/access-security
  2 go.ligato.io/cn-infra/v2/datasync/kvdbsync
  2 go.ligato.io/cn-infra/v2/datasync/syncbase
  2 go.ligato.io/cn-infra/v2/db/keyval/etcd
  2 go.ligato.io/cn-infra/v2/health/probe
  2 go.ligato.io/cn-infra/v2/rpc/prometheus
  2 go.ligato.io/cn-infra/v2/utils/safeclose
  3 go.ligato.io/cn-infra/v2/agent
  3 go.ligato.io/cn-infra/v2/rpc/grpc
  4 go.ligato.io/cn-infra/v2/datasync/resync
  4 go.ligato.io/cn-infra/v2/idxmap/mem
  4 go.ligato.io/cn-infra/v2/rpc/rest
  6 go.ligato.io/cn-infra/v2/datasync
  6 go.ligato.io/cn-infra/v2/datasync/kvdbsync/local
  6 go.ligato.io/cn-infra/v2/db/keyval
  7 go.ligato.io/cn-infra/v2/config
  9 go.ligato.io/cn-infra/v2/logging/logrus
  9 go.ligato.io/cn-infra/v2/servicelabel
  9 go.ligato.io/cn-infra/v2/utils/addrs
 12 go.ligato.io/cn-infra/v2/health/statuscheck
 23 go.ligato.io/cn-infra/v2/idxmap
 23 go.ligato.io/cn-infra/v2/infra
 90 go.ligato.io/cn-infra/v2/logging

# vpp-agent directly imports 31 packages from cn-infra
go.ligato.io/cn-infra/v2/agent
go.ligato.io/cn-infra/v2/config
go.ligato.io/cn-infra/v2/datasync
go.ligato.io/cn-infra/v2/datasync/kvdbsync
go.ligato.io/cn-infra/v2/datasync/kvdbsync/local
go.ligato.io/cn-infra/v2/datasync/msgsync
go.ligato.io/cn-infra/v2/datasync/resync
go.ligato.io/cn-infra/v2/datasync/syncbase
go.ligato.io/cn-infra/v2/db/keyval
go.ligato.io/cn-infra/v2/db/keyval/consul
go.ligato.io/cn-infra/v2/db/keyval/etcd
go.ligato.io/cn-infra/v2/db/keyval/kvproto
go.ligato.io/cn-infra/v2/db/keyval/redis
go.ligato.io/cn-infra/v2/exec/supervisor
go.ligato.io/cn-infra/v2/health/probe
go.ligato.io/cn-infra/v2/health/statuscheck
go.ligato.io/cn-infra/v2/health/statuscheck/model/status
go.ligato.io/cn-infra/v2/idxmap
go.ligato.io/cn-infra/v2/idxmap/mem
go.ligato.io/cn-infra/v2/infra
go.ligato.io/cn-infra/v2/logging
go.ligato.io/cn-infra/v2/logging/logmanager
go.ligato.io/cn-infra/v2/logging/logrus
go.ligato.io/cn-infra/v2/messaging/kafka
go.ligato.io/cn-infra/v2/rpc/grpc
go.ligato.io/cn-infra/v2/rpc/prometheus
go.ligato.io/cn-infra/v2/rpc/rest
go.ligato.io/cn-infra/v2/rpc/rest/security/model/access-security
go.ligato.io/cn-infra/v2/servicelabel
go.ligato.io/cn-infra/v2/utils/addrs
go.ligato.io/cn-infra/v2/utils/safeclose

# vpp-agent has dependency on 45 packages from cn-infra
go.ligato.io/cn-infra/v2/agent
go.ligato.io/cn-infra/v2/config
go.ligato.io/cn-infra/v2/datasync
go.ligato.io/cn-infra/v2/datasync/kvdbsync
go.ligato.io/cn-infra/v2/datasync/kvdbsync/local
go.ligato.io/cn-infra/v2/datasync/msgsync
go.ligato.io/cn-infra/v2/datasync/resync
go.ligato.io/cn-infra/v2/datasync/syncbase
go.ligato.io/cn-infra/v2/db/keyval
go.ligato.io/cn-infra/v2/db/keyval/consul
go.ligato.io/cn-infra/v2/db/keyval/etcd
go.ligato.io/cn-infra/v2/db/keyval/kvproto
go.ligato.io/cn-infra/v2/db/keyval/redis
go.ligato.io/cn-infra/v2/exec/processmanager
go.ligato.io/cn-infra/v2/exec/processmanager/status
go.ligato.io/cn-infra/v2/exec/processmanager/template
go.ligato.io/cn-infra/v2/exec/processmanager/template/model/process
go.ligato.io/cn-infra/v2/exec/supervisor
go.ligato.io/cn-infra/v2/health/probe
go.ligato.io/cn-infra/v2/health/statuscheck
go.ligato.io/cn-infra/v2/health/statuscheck/model/status
go.ligato.io/cn-infra/v2/idxmap
go.ligato.io/cn-infra/v2/idxmap/mem
go.ligato.io/cn-infra/v2/infra
go.ligato.io/cn-infra/v2/logging
go.ligato.io/cn-infra/v2/logging/logmanager
go.ligato.io/cn-infra/v2/logging/logrus
go.ligato.io/cn-infra/v2/logging/measure
go.ligato.io/cn-infra/v2/logging/measure/model/apitrace
go.ligato.io/cn-infra/v2/messaging
go.ligato.io/cn-infra/v2/messaging/kafka
go.ligato.io/cn-infra/v2/messaging/kafka/client
go.ligato.io/cn-infra/v2/messaging/kafka/mux
go.ligato.io/cn-infra/v2/rpc/grpc
go.ligato.io/cn-infra/v2/rpc/prometheus
go.ligato.io/cn-infra/v2/rpc/rest
go.ligato.io/cn-infra/v2/rpc/rest/security
go.ligato.io/cn-infra/v2/rpc/rest/security/model/access-security
go.ligato.io/cn-infra/v2/servicelabel
go.ligato.io/cn-infra/v2/utils/addrs
go.ligato.io/cn-infra/v2/utils/clienttls
go.ligato.io/cn-infra/v2/utils/once
go.ligato.io/cn-infra/v2/utils/ratelimit
go.ligato.io/cn-infra/v2/utils/redact
go.ligato.io/cn-infra/v2/utils/safeclose

kiknos

imports 20, depends on 36
# kiknos imports following cn-infra packages
  1 go.ligato.io/cn-infra/v2/config
  1 go.ligato.io/cn-infra/v2/exec/supervisor
  2 go.ligato.io/cn-infra/v2/datasync/resync
  3 go.ligato.io/cn-infra/v2/datasync/kvdbsync
  3 go.ligato.io/cn-infra/v2/datasync/kvdbsync/local
  3 go.ligato.io/cn-infra/v2/db/cryptodata
  3 go.ligato.io/cn-infra/v2/health/probe
  3 go.ligato.io/cn-infra/v2/health/statuscheck
  3 go.ligato.io/cn-infra/v2/logging/logmanager
  4 go.ligato.io/cn-infra/v2/datasync
  4 go.ligato.io/cn-infra/v2/logging/logrus
  5 go.ligato.io/cn-infra/v2/agent
  5 go.ligato.io/cn-infra/v2/db/keyval
  5 go.ligato.io/cn-infra/v2/db/keyval/etcd
  5 go.ligato.io/cn-infra/v2/rpc/prometheus
  6 go.ligato.io/cn-infra/v2/rpc/grpc
  7 go.ligato.io/cn-infra/v2/infra
  7 go.ligato.io/cn-infra/v2/rpc/rest
  9 go.ligato.io/cn-infra/v2/servicelabel
 16 go.ligato.io/cn-infra/v2/logging

# kiknos directly imports 20 packages from cn-infra
go.ligato.io/cn-infra/v2/agent
go.ligato.io/cn-infra/v2/config
go.ligato.io/cn-infra/v2/datasync
go.ligato.io/cn-infra/v2/datasync/kvdbsync
go.ligato.io/cn-infra/v2/datasync/kvdbsync/local
go.ligato.io/cn-infra/v2/datasync/resync
go.ligato.io/cn-infra/v2/db/cryptodata
go.ligato.io/cn-infra/v2/db/keyval
go.ligato.io/cn-infra/v2/db/keyval/etcd
go.ligato.io/cn-infra/v2/exec/supervisor
go.ligato.io/cn-infra/v2/health/probe
go.ligato.io/cn-infra/v2/health/statuscheck
go.ligato.io/cn-infra/v2/infra
go.ligato.io/cn-infra/v2/logging
go.ligato.io/cn-infra/v2/logging/logmanager
go.ligato.io/cn-infra/v2/logging/logrus
go.ligato.io/cn-infra/v2/rpc/grpc
go.ligato.io/cn-infra/v2/rpc/prometheus
go.ligato.io/cn-infra/v2/rpc/rest
go.ligato.io/cn-infra/v2/servicelabel

# kiknos has dependency on 36 packages from cn-infra
go.ligato.io/cn-infra/v2/agent
go.ligato.io/cn-infra/v2/config
go.ligato.io/cn-infra/v2/datasync
go.ligato.io/cn-infra/v2/datasync/kvdbsync
go.ligato.io/cn-infra/v2/datasync/kvdbsync/local
go.ligato.io/cn-infra/v2/datasync/resync
go.ligato.io/cn-infra/v2/datasync/syncbase
go.ligato.io/cn-infra/v2/db/cryptodata
go.ligato.io/cn-infra/v2/db/keyval
go.ligato.io/cn-infra/v2/db/keyval/etcd
go.ligato.io/cn-infra/v2/db/keyval/kvproto
go.ligato.io/cn-infra/v2/exec/processmanager
go.ligato.io/cn-infra/v2/exec/processmanager/status
go.ligato.io/cn-infra/v2/exec/processmanager/template
go.ligato.io/cn-infra/v2/exec/processmanager/template/model/process
go.ligato.io/cn-infra/v2/exec/supervisor
go.ligato.io/cn-infra/v2/health/probe
go.ligato.io/cn-infra/v2/health/statuscheck
go.ligato.io/cn-infra/v2/health/statuscheck/model/status
go.ligato.io/cn-infra/v2/idxmap
go.ligato.io/cn-infra/v2/idxmap/mem
go.ligato.io/cn-infra/v2/infra
go.ligato.io/cn-infra/v2/logging
go.ligato.io/cn-infra/v2/logging/logmanager
go.ligato.io/cn-infra/v2/logging/logrus
go.ligato.io/cn-infra/v2/logging/measure
go.ligato.io/cn-infra/v2/logging/measure/model/apitrace
go.ligato.io/cn-infra/v2/rpc/grpc
go.ligato.io/cn-infra/v2/rpc/prometheus
go.ligato.io/cn-infra/v2/rpc/rest
go.ligato.io/cn-infra/v2/rpc/rest/security
go.ligato.io/cn-infra/v2/rpc/rest/security/model/access-security
go.ligato.io/cn-infra/v2/servicelabel
go.ligato.io/cn-infra/v2/utils/addrs
go.ligato.io/cn-infra/v2/utils/once
go.ligato.io/cn-infra/v2/utils/safeclose

Used commands:

# count of imports for cn-infra packages
go list -f '{{if .Module}}{{if not .Standard}}{{range .Imports}}{{printf "%s\n" .}}{{end}}{{end}}{{end}}' $(go list ./...) | grep cn-infra | sort | uniq -c | sort -h

# list package dependencies (including transitive) for cn-infra
go list ./... | xargs go list -f '{{range .Imports}}{{printf "%s\n" .}}{{end}}' | grep cn-infra | sort | uniq

# list direct imports of cn-infra packages
go list ./... | xargs go list -f '{{range .Imports}}{{printf "%s\n" .}}{{end}}' | grep cn-infra | sort | uniq

Components

Infra

  • config
  • logging
  • runtime/app/cli
  • plugin/service
  • debug/prof
  • metrics/stats
  • version

Plugins

Plugins are components defined by generic API that support pluggable implementations.

  • kvstore plugins
    • bolt, etcd, consul, filedb, redis, ..
  • sqldb plugins
    • cassandra, sqlite, ..
  • datasync plugins
    • kvstore, rpc, .. ???
  • rpc plugins
    • grpc, rest, .. ???
  • exec plugins
    • supervisor, .. ???

Goals

  • Reorganize package locations

    • agent => infra/app or infra/runtime+infra/version?
    • config => infra/cfg?
    • logging => infra/log?
    • servicelabel => infra/service or infra/label?
    • health/probe => infra/health?
    • health/status => infra/health?
    • db/keyval => kvstore or db/kvstore?
    • db/cryptodata => security/crypto?
    • rpc/rest/security => security/rest?
    • utils/redact => security/redact?
    • utils/ratelimit => security/ratelimit?
    • rpc/prometheus => telemetry/prometheus
    • idxmap => utils/idxmap
  • Get rid of magic behaviours

    • plugin dependency lookup should have more explicit behaviour
  • Redefine complex interfaces into simpler ones

    • kvstore package should define cleaner KV store API (no proto or datasync)
    • datasync should use single channel for data (instead of change/resync channels)
  • Look for ways to avoid forcing cn-infra essentials to users

    • Get rid of plugin overuse
  • Decouple packages with mixed responsibilities into smaller pieces

    • agent package should be separated into: app/version/runtime/infra/..?
  • Remove unused code / parts of repo

    • docker directory can be possibly removed
    • utils/safeclose should be removed (promotes unidiomatic code)
    • logging/measure is not used
  • Move common packages from vpp-agent repository

    • pkg/debug => infra/debug
    • pkg/version => infra/version
    • plugins/kvscheduler => processor???
Clone this wiki locally