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.

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

Introducing FeatureGates and global feature config #344

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
69 changes: 60 additions & 9 deletions cmd/sriovdp/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,14 @@ package main

import (
"flag"
"fmt"
"os"
"os/signal"
"syscall"

"github.com/k8snetworkplumbingwg/sriov-network-device-plugin/pkg/config"
Copy link
Member

Choose a reason for hiding this comment

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

Nit: Separate imports according to convention [1]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

"github.com/k8snetworkplumbingwg/sriov-network-device-plugin/pkg/features"

"github.com/golang/glog"
)

Expand All @@ -33,6 +37,8 @@ func flagInit(cp *cliParams) {
"JSON device pool config file location")
flag.StringVar(&cp.resourcePrefix, "resource-prefix", "intel.com",
"resource name prefix used for K8s extended resource")
flag.StringVar(&cp.featureGates, "feature-gates", "",
"enables or disables selected features")
}

func main() {
Expand All @@ -57,24 +63,50 @@ func main() {
glog.Fatalf("Exiting.. one or more invalid configuration(s) given")
return
}

config.NewConfig()

cfg, err := config.GetConfig()
if err != nil {
glog.Fatalf("error while getting config: %v", err)
return
}

// Read global config
if err := cfg.ReadConfig(cp.configFile); err != nil {
glog.Error(err)
return
}

fg, err := prepareFeaturegates()
if err != nil {
glog.Fatalf("error while getting feature gate: %v", err)
return
}

// Set FeatureGates with ConfigMap
if err := fg.SetFromMap(cfg.FeatureGates); err != nil {
glog.Error(err)
return
}

// Set FeatureGates with CLI arguments
if err := fg.SetFromString(cp.featureGates); err != nil {
glog.Error(err)
return
}

glog.Infof("Discovering host devices")
if err := rm.discoverHostDevices(); err != nil {
glog.Errorf("error discovering host devices%v", err)
return
}

glog.Infof("Initializing resource servers")
if err := rm.initServers(); err != nil {
glog.Errorf("error initializing resource servers %v", err)
if err := startServers(rm); err != nil {
glog.Error(err)
return
}

glog.Infof("Starting all servers...")
if err := rm.startAllServers(); err != nil {
glog.Errorf("error starting resource servers %v\n", err)
return
}
glog.Infof("All servers started.")
glog.Infof("Listening for term signals")
// respond to syscalls for termination
sigCh := make(chan os.Signal, 1)
Expand All @@ -87,3 +119,22 @@ func main() {
glog.Errorf("stopping servers produced error: %s", err.Error())
}
}

func prepareFeaturegates() (*features.FeatureGate, error) {
features.NewFeatureGate()
return features.GetFeatureGate()
}

func startServers(rm *resourceManager) error {
glog.Infof("Initializing resource servers")
if err := rm.initServers(); err != nil {
return fmt.Errorf("error initializing resource servers %w", err)
}

glog.Infof("Starting all servers...")
if err := rm.startAllServers(); err != nil {
return fmt.Errorf("error starting resource servers %w", err)
}
glog.Infof("All servers started.")
return nil
}
1 change: 1 addition & 0 deletions cmd/sriovdp/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ const (
type cliParams struct {
configFile string
resourcePrefix string
featureGates string
}

type resourceManager struct {
Expand Down
6 changes: 5 additions & 1 deletion cmd/sriovdp/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,11 @@ var _ = Describe("Resource manager", func() {
"drivers": ["vfio-pci"]
}
}
]
],
"featureGates": {
"testFeatureOne": true,
"testFeatureTwo": false
}
}`), 0644)
if testErr != nil {
panic(testErr)
Expand Down
69 changes: 69 additions & 0 deletions pkg/config/config.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
package config

import (
"encoding/json"
"fmt"
"os"
)

var cfg *Config

// Config defines config structure
type Config struct {
FeatureGates map[string]bool `json:"featureGates,omitempty"`
}

// NewConfig creates new config
func NewConfig() {
if cfg == nil {
cfg = newConfig()
}
}

// GetConfig returns config
func GetConfig() (*Config, error) {
var err error
if cfg == nil {
err = fmt.Errorf("config was not initialized")
}
return cfg, err
}

func newConfig() *Config {
if cfg != nil {
return cfg
}
newCfg := &Config{}
newCfg.FeatureGates = make(map[string]bool)
return newCfg
}

// ReadConfig loads config
func (cfg *Config) ReadConfig(configFile string) error {
allCfg := make(map[string]json.RawMessage)
rawBytes, err := os.ReadFile(configFile)

if err != nil {
return fmt.Errorf("error reading file %s, %v", configFile, err)
}

if err = json.Unmarshal(rawBytes, &allCfg); err != nil {
return fmt.Errorf("error unmarshalling raw bytes %v", err)
}

fgMap := make(map[string]bool)

if _, exists := allCfg["featureGates"]; !exists {
return fmt.Errorf("no config for feature gate present")
}

if err = json.Unmarshal(allCfg["featureGates"], &fgMap); err != nil {
Copy link
Member

Choose a reason for hiding this comment

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

shouldn't you check if "featureGates" key is in the map before indexing? Potential panic if it isn't available.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

return fmt.Errorf("error unmarshalling raw bytes %v", err)
}

for k, v := range fgMap {
cfg.FeatureGates[k] = v
}

return nil
}
151 changes: 151 additions & 0 deletions pkg/features/features.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,151 @@
package features

import (
"fmt"
"strconv"
"strings"

"github.com/golang/glog"
)

// FeatureGate is feature gate 'manager' to be used
var fg *FeatureGate

// List of supported feature maturity levels
const (
// Alpha - alpha version
Alpha = string("ALPHA")

// GA - general availability
GA = string("GA")

// Deprecated - feature that will be deprecated in 2 releases
Deprecated = string("DEPRECATED")

splitLength = 2
)

// List of supported features
const (
// AlphaFeature - description
AlphaFeature string = "alphaFeature"

// BetaFeature - description
BetaFeature string = "betaFeature"

// GaFeature - description
GaFeature string = "gaFeature"

// DeprecatedFeature - description
DeprecatedFeature string = "deprecatedFeature"
)

type featureSpec struct {
// Default is the default enablement state for the feature
Default bool
// Maturity indicates the maturity level of the feature
Maturity string
}

var defaultSriovDpFeatureGates = map[string]featureSpec{
AlphaFeature: {Default: false, Maturity: Alpha},
GaFeature: {Default: true, Maturity: GA},
Copy link
Member

Choose a reason for hiding this comment

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

GaFeature -> GAFeature

DeprecatedFeature: {Default: false, Maturity: Deprecated},
}

// FeatureGate defines FeatureGate structure
type FeatureGate struct {
knownFeatures map[string]featureSpec
enabled map[string]bool
}

// NewFeatureGate creates new FeatureGate if it does not exist yet
func NewFeatureGate() {
fg = newFeatureGate()
}

// GetFeatureGate returns current feature gate
func GetFeatureGate() (*FeatureGate, error) {
var err error
if fg == nil {
err = fmt.Errorf("feature gate object was not initialized")
}
return fg, err
}

func newFeatureGate() *FeatureGate {
if fg != nil {
return fg
}
fg := &FeatureGate{}
fg.knownFeatures = make(map[string]featureSpec)
fg.enabled = make(map[string]bool)

for k, v := range defaultSriovDpFeatureGates {
fg.knownFeatures[k] = v
}

for k, v := range fg.knownFeatures {
fg.enabled[k] = v.Default
}
return fg
}

// Enabled returns enabelement status of the provided feature
Copy link
Member

Choose a reason for hiding this comment

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

enabelement -> enablement

func (fg *FeatureGate) Enabled(featureName string) bool {
return fg.enabled[featureName]
}

func (fg *FeatureGate) isFeatureSupported(featureName string) bool {
_, exists := fg.knownFeatures[featureName]
return exists
}

func (fg *FeatureGate) set(featureName string, status bool) error {
if !fg.isFeatureSupported(featureName) {
return fmt.Errorf("feature %s is not supported", featureName)
}
fg.enabled[featureName] = status
if status && fg.knownFeatures[featureName].Maturity == Deprecated {
glog.Warningf("WARNING: Feature %s will be deprecated soon", featureName)
}
return nil
}

// SetFromMap sets the enablement status of featuers accordig to a map
Copy link
Member

Choose a reason for hiding this comment

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

two typos featuers & accordig

func (fg *FeatureGate) SetFromMap(valuesToSet map[string]bool) error {
for k, v := range valuesToSet {
if err := fg.set(k, v); err != nil {
return err
}
}
return nil
}

// SetFromString converts config string to map and sets the enablement status of the selected features
// copied from k8s and slightly changed - TBC?
func (fg *FeatureGate) SetFromString(value string) error {
featureMap := make(map[string]bool)
for _, s := range strings.Split(value, ",") {
if s == "" {
continue
}
splitted := strings.Split(s, "=")
key := strings.TrimSpace(splitted[0])
if len(splitted) != splitLength {
if len(splitted) > splitLength {
return fmt.Errorf("too many values for %s", key)
}
return fmt.Errorf("enablement value for %s is missing", key)
}

val := strings.TrimSpace(splitted[1])
boolVal, err := strconv.ParseBool(val)
if err != nil {
return fmt.Errorf("error while processing %s=%s, err: %v", key, val, err)
}

featureMap[key] = boolVal
}
return fg.SetFromMap(featureMap)
}
4 changes: 2 additions & 2 deletions pkg/netdevice/netInfoProviders.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import (
)

/*
rdmaInfoProvider provides the RDMA information
rdmaInfoProvider provides the RDMA information
*/
type rdmaInfoProvider struct {
rdmaSpec types.RdmaSpec
Expand Down Expand Up @@ -58,7 +58,7 @@ func (rip *rdmaInfoProvider) GetMounts() []*pluginapi.Mount {
}

/*
VhostNetInfoProvider wraps any DeviceInfoProvider and adds a vhost-net device
VhostNetInfoProvider wraps any DeviceInfoProvider and adds a vhost-net device
*/
type vhostNetInfoProvider struct {
}
Expand Down
3 changes: 2 additions & 1 deletion pkg/netdevice/netResourcePool.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,8 @@ func (rp *netResourcePool) GetDeviceSpecs(deviceIDs []string) []*pluginapi.Devic
}

// StoreDeviceInfoFile stores the Device Info files according to the
// k8snetworkplumbingwg/device-info-spec
//
// k8snetworkplumbingwg/device-info-spec
func (rp *netResourcePool) StoreDeviceInfoFile(resourceNamePrefix string) error {
var devInfo nettypes.DeviceInfo
for id, dev := range rp.GetDevicePool() {
Expand Down
7 changes: 5 additions & 2 deletions pkg/netdevice/vdpa.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,11 @@ import (
"github.com/k8snetworkplumbingwg/sriov-network-device-plugin/pkg/utils"
)

/*vdpaTypeToDriver translates vdpaTypes (as specified in the netDevice selectors)
to vdpa bus drivers*/
/*
vdpaTypeToDriver translates vdpaTypes (as specified in the netDevice selectors)

to vdpa bus drivers
*/
var supportedVdpaTypes = map[types.VdpaType]string{
types.VdpaVirtioType: vdpa.VirtioVdpaDriver,
types.VdpaVhostType: vdpa.VhostVdpaDriver,
Expand Down
Loading