Skip to content

Commit

Permalink
refactor that error is last return value and fix show bug if node not…
Browse files Browse the repository at this point in the history
… in onfig
  • Loading branch information
severindellsperger committed May 15, 2024
1 parent 6a6f066 commit 429cd74
Show file tree
Hide file tree
Showing 14 changed files with 109 additions and 112 deletions.
2 changes: 1 addition & 1 deletion clab-telemetry-linker/cmd/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ var deleteCmd = &cobra.Command{
Use: "delete",
Short: "Delete impairments on a containerlab interface",
Run: func(cmd *cobra.Command, args []string) {
err, defaultConfig := config.NewDefaultConfig()
defaultConfig, err := config.NewDefaultConfig()
if err != nil {
log.Fatalf("Error reading/creating config: %v\n", err)
}
Expand Down
2 changes: 1 addition & 1 deletion clab-telemetry-linker/cmd/set.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ var setCmd = &cobra.Command{
Use: "set",
Short: "Set impairments on a containerlab interface",
Run: func(cmd *cobra.Command, args []string) {
err, defaultConfig := config.NewDefaultConfig()
defaultConfig, err := config.NewDefaultConfig()
if err != nil {
log.Fatalf("Error reading/creating config: %v\n", err)
}
Expand Down
7 changes: 2 additions & 5 deletions clab-telemetry-linker/cmd/show.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,12 @@ import (

var showCmd = &cobra.Command{
Use: "show",
Short: "Show impairments on a containerlab interface",
Short: "Show impairments on a containerlab node",
Run: func(cmd *cobra.Command, args []string) {
err, defaultConfig := config.NewDefaultConfig()
defaultConfig, err := config.NewDefaultConfig()
if err != nil {
log.Fatalf("Error reading/creating config: %v\n", err)
}
if defaultConfig.GetValue("nodes."+Node) == "" {
log.Fatalf("Node %s not found in config\n", Node)
}
helper := helpers.NewDefaultHelper()
command := command.NewDefaultShowCommand(Node, defaultConfig.GetValue(helper.GetDefaultClabNameKey()))
manager := impairments.NewDefaultViewer(Node, command)
Expand Down
2 changes: 1 addition & 1 deletion clab-telemetry-linker/cmd/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ var startCmd = &cobra.Command{
Use: "start",
Short: "Start processing the telemetry data",
Run: func(cmd *cobra.Command, args []string) {
err, defaultConfig := config.NewDefaultConfig()
defaultConfig, err := config.NewDefaultConfig()
if err != nil {
log.Fatalf("Error creating config: %v\n", err)
}
Expand Down
30 changes: 15 additions & 15 deletions pkg/config/default.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ type DefaultConfig struct {
}

func (config *DefaultConfig) setUserHome() error {
if err, userHome := config.helper.GetUserHome(); err != nil {
if userHome, err := config.helper.GetUserHome(); err != nil {
return err
} else {
config.userHome = userHome
Expand Down Expand Up @@ -73,15 +73,15 @@ func (config *DefaultConfig) setClabName(clabName string) error {
return nil
}

func (config *DefaultConfig) doesConfigExist() (error, bool) {
func (config *DefaultConfig) doesConfigExist() (bool, error) {
config.log.Debugln("Check if config file exists:", config.fullfileLocation)
if _, err := os.Stat(config.fullfileLocation); err != nil {
if os.IsNotExist(err) {
return nil, false
return false, nil
}
return fmt.Errorf("Unable to check if config file exists: %v", err), false
return false, fmt.Errorf("Unable to check if config file exists: %v", err)
}
return nil, true
return true, nil
}

func (config *DefaultConfig) createConfig() error {
Expand All @@ -108,7 +108,7 @@ func (config *DefaultConfig) readConfig() error {
}

func (config *DefaultConfig) InitConfig() error {
err, exist := config.doesConfigExist()
exist, err := config.doesConfigExist()
if err != nil {
return err
}
Expand Down Expand Up @@ -174,32 +174,32 @@ func (config *DefaultConfig) WriteConfig() error {
return nil
}

func CreateDefaultConfig(configFileName, clabName, clabNameKey string, helper helpers.Helper) (error, *DefaultConfig) {
func CreateDefaultConfig(configFileName, clabName, clabNameKey string, helper helpers.Helper) (*DefaultConfig, error) {
defaultConfig := &DefaultConfig{
log: logging.DefaultLogger.WithField("subsystem", Subsystem),
koanfInstance: koanf.New("."),
clabNameKey: helper.GetDefaultClabNameKey(),
helper: helper,
}
if err := defaultConfig.setUserHome(); err != nil {
return err, nil
return nil, err
}
defaultConfig.setConfigFileName(configFileName)
defaultConfig.setConfigPath()
defaultConfig.setConfigFileLocation()
if err := defaultConfig.setClabName(clabName); err != nil {
return err, nil
return nil, err
}
if err := defaultConfig.InitConfig(); err != nil {
return err, nil
return nil, err
}
return nil, defaultConfig
return defaultConfig, nil
}

func NewDefaultConfig() (error, *DefaultConfig) {
err, defaultConfig := CreateDefaultConfig("", "", "", helpers.NewDefaultHelper())
func NewDefaultConfig() (*DefaultConfig, error) {
defaultConfig, err := CreateDefaultConfig("", "", "", helpers.NewDefaultHelper())
if err != nil {
return err, nil
return nil, err
}
return nil, defaultConfig
return defaultConfig, nil
}
20 changes: 10 additions & 10 deletions pkg/config/default_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,12 @@ func TestDefaultConfig_setUserHome(t *testing.T) {
helper := helpers.NewMockHelper(ctrl)
config := &DefaultConfig{helper: helper}
if !tt.wantError {
helper.EXPECT().GetUserHome().Return(nil, tt.want)
helper.EXPECT().GetUserHome().Return(tt.want, nil)
assert.NoError(t, config.setUserHome())
assert.Equal(t, tt.want, config.userHome)
}
if tt.wantError {
helper.EXPECT().GetUserHome().Return(errors.New("artificial error"), "")
helper.EXPECT().GetUserHome().Return("", errors.New("artificial error"))
assert.Error(t, config.setUserHome())
}
})
Expand Down Expand Up @@ -225,7 +225,7 @@ func TestDefaultConfig_doesConfigExists(t *testing.T) {
log: logging.DefaultLogger.WithField("subsystem", "config_test"),
fullfileLocation: tt.fields.fullfileLocation,
}
err, exists := config.doesConfigExist()
exists, err := config.doesConfigExist()
if tt.wantError {
assert.Error(t, err)
} else {
Expand Down Expand Up @@ -590,15 +590,15 @@ func TestDefaultConfig_createDefaultConfig(t *testing.T) {
helper.EXPECT().GetDefaultClabNameKey().Return(tt.want.clabNameKey).AnyTimes()
helper.EXPECT().GetDefaultClabName().Return(tt.want.clabName).AnyTimes()
if tt.want.userHome == "" {
helper.EXPECT().GetUserHome().Return(errors.New("artificial error"), "")
helper.EXPECT().GetUserHome().Return("", errors.New("artificial error"))
} else {
helper.EXPECT().GetUserHome().Return(nil, tt.want.userHome)
helper.EXPECT().GetUserHome().Return(tt.want.userHome, nil)
}
if tt.wantError {
err, _ := CreateDefaultConfig(tt.want.fileName, tt.want.clabName, tt.want.clabNameKey, helper)
_, err := CreateDefaultConfig(tt.want.fileName, tt.want.clabName, tt.want.clabNameKey, helper)
assert.Error(t, err)
} else {
err, defaultConfig := CreateDefaultConfig(tt.want.fileName, tt.want.clabName, tt.want.clabNameKey, helper)
defaultConfig, err := CreateDefaultConfig(tt.want.fileName, tt.want.clabName, tt.want.clabNameKey, helper)
assert.NoError(t, err)
assert.NotNil(t, defaultConfig)
assert.Equal(t, tt.want.userHome, defaultConfig.userHome)
Expand All @@ -623,7 +623,7 @@ func TestDefaultConfig_NewDefaultConfig(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
err, config := NewDefaultConfig()
config, err := NewDefaultConfig()
if tt.wantError {
assert.Error(t, err)
assert.Nil(t, config)
Expand Down Expand Up @@ -657,9 +657,9 @@ func TestDefaultConfig_WatchConfigChange(t *testing.T) {
helper := helpers.NewMockHelper(ctrl)
helper.EXPECT().GetDefaultClabNameKey().Return(tt.want.clabNameKey).AnyTimes()
helper.EXPECT().GetDefaultClabName().Return(tt.want.clabName).AnyTimes()
helper.EXPECT().GetUserHome().Return(nil, tt.want.userHome)
helper.EXPECT().GetUserHome().Return(tt.want.userHome, nil)

err, defaultConfig := CreateDefaultConfig(tt.want.fileName, tt.want.clabName, tt.want.clabNameKey, helper)
defaultConfig, err := CreateDefaultConfig(tt.want.fileName, tt.want.clabName, tt.want.clabNameKey, helper)
assert.NoError(t, err)
err = defaultConfig.WatchConfigChange()
assert.NoError(t, os.WriteFile(defaultConfig.fullfileLocation, []byte("clab-name: new-name"), 0644))
Expand Down
44 changes: 22 additions & 22 deletions pkg/consumer/kafka.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,17 +68,17 @@ func (consumer *KafkaConsumer) Init() error {
return nil
}

func (consumer *KafkaConsumer) UnmarshalTelemetryMessage(message *sarama.ConsumerMessage) (error, *TelemetryMessage) {
func (consumer *KafkaConsumer) UnmarshalTelemetryMessage(message *sarama.ConsumerMessage) (*TelemetryMessage, error) {
consumer.log.Debugln("Received JSON message: ", string(message.Value))
var telemetryMessage TelemetryMessage
if err := json.Unmarshal([]byte(message.Value), &telemetryMessage); err != nil {
consumer.log.Debugln("Error unmarshalling message: ", err)
return err, nil
return nil, err
}
return nil, &telemetryMessage
return &telemetryMessage, nil
}

func (consumer *KafkaConsumer) UnmarshalDelayMessage(telemetryMessage TelemetryMessage) (error, *DelayMessage) {
func (consumer *KafkaConsumer) UnmarshalDelayMessage(telemetryMessage TelemetryMessage) (*DelayMessage, error) {
delayMessage := DelayMessage{TelemetryMessage: telemetryMessage}

fields := map[string]*uint32{
Expand All @@ -90,74 +90,74 @@ func (consumer *KafkaConsumer) UnmarshalDelayMessage(telemetryMessage TelemetryM
for key, field := range fields {
value, ok := telemetryMessage.Fields[key].(float64)
if !ok {
return fmt.Errorf("unable to convert %s to float64", key), nil
return nil, fmt.Errorf("unable to convert %s to float64", key)
}
*field = uint32(value)
}
return nil, &delayMessage
return &delayMessage, nil
}

func (consumer *KafkaConsumer) UnmarshalIsisMessage(telemetryMessage TelemetryMessage) (error, []Message) {
func (consumer *KafkaConsumer) UnmarshalIsisMessage(telemetryMessage TelemetryMessage) ([]Message, error) {
var messages []Message
var err error
var msg Message

if telemetryMessage.Fields["interface_status_and_data/enabled/packet_loss_percentage"] != nil {
err, msg = consumer.UnmarshalLossMessage(telemetryMessage)
msg, err = consumer.UnmarshalLossMessage(telemetryMessage)
if err != nil {
return err, nil
return nil, err
}
messages = append(messages, msg)
}

if telemetryMessage.Fields["interface_status_and_data/enabled/bandwidth"] != nil {
err, msg = consumer.UnmarshalBandwidthMessage(telemetryMessage)
msg, err = consumer.UnmarshalBandwidthMessage(telemetryMessage)
if err != nil {
return err, nil
return nil, err
}
messages = append(messages, msg)
}

if len(messages) == 0 {
return fmt.Errorf("Received unknown ISIS message: %v", telemetryMessage), nil
return nil, fmt.Errorf("Received unknown ISIS message: %v", telemetryMessage)
}

return nil, messages
return messages, nil
}

func (consumer *KafkaConsumer) UnmarshalLossMessage(telemetryMessage TelemetryMessage) (error, *LossMessage) {
func (consumer *KafkaConsumer) UnmarshalLossMessage(telemetryMessage TelemetryMessage) (*LossMessage, error) {
lossMessage := LossMessage{TelemetryMessage: telemetryMessage}
value, ok := telemetryMessage.Fields["interface_status_and_data/enabled/packet_loss_percentage"].(float64)
if !ok {
return fmt.Errorf("unable to convert packet_loss_percentage to float"), nil
return nil, fmt.Errorf("unable to convert packet_loss_percentage to float")
}
lossMessage.LossPercentage = value
return nil, &lossMessage
return &lossMessage, nil
}

func (consumer *KafkaConsumer) UnmarshalBandwidthMessage(telemetryMessage TelemetryMessage) (error, *BandwidthMessage) {
func (consumer *KafkaConsumer) UnmarshalBandwidthMessage(telemetryMessage TelemetryMessage) (*BandwidthMessage, error) {
bandwidthMessage := BandwidthMessage{TelemetryMessage: telemetryMessage}
value, ok := telemetryMessage.Fields["interface_status_and_data/enabled/bandwidth"].(float64)
if !ok {
return fmt.Errorf("unable to convert bandwidth to float64"), nil
return nil, fmt.Errorf("unable to convert bandwidth to float64")
}
bandwidthMessage.Bandwidth = value
return nil, &bandwidthMessage
return &bandwidthMessage, nil
}

func (consumer *KafkaConsumer) processMessage(message *sarama.ConsumerMessage) {
err, telemetryMessage := consumer.UnmarshalTelemetryMessage(message)
telemetryMessage, err := consumer.UnmarshalTelemetryMessage(message)
if err != nil {
return
}
if telemetryMessage.Name == "performance-measurement" {
err, delayMessage := consumer.UnmarshalDelayMessage(*telemetryMessage)
delayMessage, err := consumer.UnmarshalDelayMessage(*telemetryMessage)
if err != nil {
return
}
consumer.unprocessedMsgChan <- delayMessage
} else if telemetryMessage.Name == "isis" {
err, isisMessages := consumer.UnmarshalIsisMessage(*telemetryMessage)
isisMessages, err := consumer.UnmarshalIsisMessage(*telemetryMessage)
if err != nil {
return
}
Expand Down
18 changes: 9 additions & 9 deletions pkg/consumer/kafka_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ func TestKafkaConsumer_UnmarshalTelemetryMessage(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
kafkaConsumer := NewKafkaConsumer(tt.fields.kafkaBroker, tt.fields.kafkaTopic, tt.fields.unprocessedMsgChan)
err, telemetryMsg := kafkaConsumer.UnmarshalTelemetryMessage(tt.args.message)
telemetryMsg, err := kafkaConsumer.UnmarshalTelemetryMessage(tt.args.message)
if tt.wantErr {
assert.Error(t, err)
} else {
Expand Down Expand Up @@ -420,9 +420,9 @@ func TestKafkaConsumer_UnmarshalDelayMessage(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
kafkaConsumer := NewKafkaConsumer(tt.fields.kafkaBroker, tt.fields.kafkaTopic, tt.fields.unprocessedMsgChan)
err, telemetryMsg := kafkaConsumer.UnmarshalTelemetryMessage(tt.args.message)
telemetryMsg, err := kafkaConsumer.UnmarshalTelemetryMessage(tt.args.message)
assert.NoError(t, err)
err, delayMsg := kafkaConsumer.UnmarshalDelayMessage(*telemetryMsg)
delayMsg, err := kafkaConsumer.UnmarshalDelayMessage(*telemetryMsg)
if tt.wantErr {
assert.Error(t, err)
} else {
Expand Down Expand Up @@ -571,9 +571,9 @@ func TestKafkaConsumer_UnmarshalIsisMessage(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
kafkaConsumer := NewKafkaConsumer(tt.fields.kafkaBroker, tt.fields.kafkaTopic, tt.fields.unprocessedMsgChan)
err, telemetryMsg := kafkaConsumer.UnmarshalTelemetryMessage(tt.args.message)
telemetryMsg, err := kafkaConsumer.UnmarshalTelemetryMessage(tt.args.message)
assert.NoError(t, err)
err, delayMsg := kafkaConsumer.UnmarshalIsisMessage(*telemetryMsg)
delayMsg, err := kafkaConsumer.UnmarshalIsisMessage(*telemetryMsg)
if tt.wantErr {
assert.Error(t, err)
} else {
Expand Down Expand Up @@ -658,9 +658,9 @@ func TestKafkaConsumer_UnmarshalLossMessage(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
kafkaConsumer := NewKafkaConsumer(tt.fields.kafkaBroker, tt.fields.kafkaTopic, tt.fields.unprocessedMsgChan)
err, telemetryMsg := kafkaConsumer.UnmarshalTelemetryMessage(tt.args.message)
telemetryMsg, err := kafkaConsumer.UnmarshalTelemetryMessage(tt.args.message)
assert.NoError(t, err)
err, isisMsg := kafkaConsumer.UnmarshalLossMessage(*telemetryMsg)
isisMsg, err := kafkaConsumer.UnmarshalLossMessage(*telemetryMsg)
if tt.wantErr {
assert.Error(t, err)
} else {
Expand Down Expand Up @@ -746,9 +746,9 @@ func TestKafkaConsumer_UnmarshalBandwidthMessage(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
kafkaConsumer := NewKafkaConsumer(tt.fields.kafkaBroker, tt.fields.kafkaTopic, tt.fields.unprocessedMsgChan)
err, telemetryMsg := kafkaConsumer.UnmarshalTelemetryMessage(tt.args.message)
telemetryMsg, err := kafkaConsumer.UnmarshalTelemetryMessage(tt.args.message)
assert.NoError(t, err)
err, bwMsg := kafkaConsumer.UnmarshalBandwidthMessage(*telemetryMsg)
bwMsg, err := kafkaConsumer.UnmarshalBandwidthMessage(*telemetryMsg)
if tt.wantErr {
assert.Error(t, err)
} else {
Expand Down
8 changes: 4 additions & 4 deletions pkg/helpers/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import (

type Helper interface {
IsRoot() bool
GetUserHome() (error, string)
GetUserHome() (string, error)
GetDefaultClabNameKey() string
GetDefaultClabName() string
GetDefaultImpairmentsPrefix(node, interface_ string) string
Expand All @@ -24,13 +24,13 @@ func (helper *DefaultHelper) IsRoot() bool {
return os.Geteuid() == 0
}

func (helper *DefaultHelper) GetUserHome() (error, string) {
func (helper *DefaultHelper) GetUserHome() (string, error) {
username := os.Getenv("SUDO_USER")
user, err := user.Lookup(username)
if err != nil {
return fmt.Errorf("Unable to find userhome for user %q: %v", username, err), ""
return "", fmt.Errorf("Unable to find userhome for user %q: %v", username, err)
}
return nil, user.HomeDir
return user.HomeDir, nil
}

func (helper *DefaultHelper) GetDefaultClabNameKey() string {
Expand Down
Loading

0 comments on commit 429cd74

Please sign in to comment.