Skip to content

Refactor

Jump to bottom
Ondrej Fabry edited this page Apr 6, 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

  • 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 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 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:

# 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