Skip to content

Commit

Permalink
Suppress nagging error logging.
Browse files Browse the repository at this point in the history
Remove debug logging.
Incorporate PR feedback.
Change error name per review.

Signed-off-by: James Munson <james.munson@suse.com>
  • Loading branch information
james-munson authored and David Ko committed Oct 26, 2023
1 parent 3d14187 commit 380c0c3
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 18 deletions.
2 changes: 1 addition & 1 deletion controller/setting_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -665,7 +665,7 @@ func (sc *SettingController) updateCNI() error {
}

if !volumesDetached {
return errors.Errorf("failed to apply %v setting to Longhorn workloads when there are attached volumes", types.SettingNameStorageNetwork)
return &types.ErrorInvalidState{Reason: fmt.Sprintf("failed to apply %v setting to Longhorn workloads when there are attached volumes", types.SettingNameStorageNetwork)}
}

nadAnnot := string(types.CNIAnnotationNetworks)
Expand Down
4 changes: 4 additions & 0 deletions controller/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,10 @@ func createOrUpdateAttachmentTicket(va *longhorn.VolumeAttachment, ticketID, nod
}

func handleReconcileErrorLogging(logger logrus.FieldLogger, err error, mesg string) {
if types.ErrorIsInvalidState(err) {
return
}

if apierrors.IsConflict(err) {
logger.WithError(err).Debug(mesg)
} else {
Expand Down
41 changes: 24 additions & 17 deletions datastore/longhorn.go
Original file line number Diff line number Diff line change
Expand Up @@ -305,39 +305,33 @@ func (s *DataStore) ValidateSetting(name, value string) (err error) {
}
}
case types.SettingNameTaintToleration:
list, err := s.ListVolumesRO()
volumesDetached, err := s.AreAllVolumesDetachedState()
if err != nil {
return errors.Wrapf(err, "failed to list volumes before modifying toleration setting")
}
for _, v := range list {
if v.Status.State != longhorn.VolumeStateDetached {
return fmt.Errorf("cannot modify toleration setting before all volumes are detached")
}
if !volumesDetached {
return &types.ErrorInvalidState{Reason: fmt.Sprintf("cannot modify toleration setting before all volumes are detached")}
}
case types.SettingNameSystemManagedComponentsNodeSelector:
list, err := s.ListVolumesRO()
volumesDetached, err := s.AreAllVolumesDetachedState()
if err != nil {
return errors.Wrapf(err, "failed to list volumes before modifying node selector for managed components setting")
}
for _, v := range list {
if v.Status.State != longhorn.VolumeStateDetached {
return fmt.Errorf("cannot modify node selector for managed components setting before all volumes are detached")
}
if !volumesDetached {
return &types.ErrorInvalidState{Reason: fmt.Sprintf("cannot modify node selector for managed components setting before all volumes are detached")}
}
case types.SettingNamePriorityClass:
if value != "" {
if _, err := s.GetPriorityClass(value); err != nil {
return errors.Wrapf(err, "failed to get priority class %v before modifying priority class setting", value)
}
}
list, err := s.ListVolumesRO()
volumesDetached, err := s.AreAllVolumesDetachedState()
if err != nil {
return errors.Wrapf(err, "failed to list volumes before modifying priority class setting")
}
for _, v := range list {
if v.Status.State != longhorn.VolumeStateDetached {
return fmt.Errorf("cannot modify priority class setting before all volumes are detached")
}
if !volumesDetached {
return &types.ErrorInvalidState{Reason: fmt.Sprintf("cannot modify priority class setting before all volumes are detached")}
}
case types.SettingNameGuaranteedInstanceManagerCPU:
guaranteedInstanceManagerCPU, err := s.GetSetting(types.SettingNameGuaranteedInstanceManagerCPU)
Expand All @@ -356,7 +350,7 @@ func (s *DataStore) ValidateSetting(name, value string) (err error) {
return errors.Wrapf(err, "failed to check volume detachment for %v setting update", name)
}
if !volumesDetached {
return errors.Errorf("cannot apply %v setting to Longhorn workloads when there are attached volumes", name)
return &types.ErrorInvalidState{Reason: fmt.Sprintf("cannot apply %v setting to Longhorn workloads when there are attached volumes", name)}
}
case types.SettingNameV2DataEngine:
old, err := s.GetSetting(types.SettingNameV2DataEngine)
Expand All @@ -383,7 +377,7 @@ func (s *DataStore) ValidateV2DataEngine(v2DataEngineEnabled bool) error {
return errors.Wrapf(err, "failed to check volume detachment for %v setting update", types.SettingNameV2DataEngine)
}
if !volumesDetached {
return errors.Errorf("cannot apply %v setting to Longhorn workloads when there are attached volumes", types.SettingNameV2DataEngine)
return &types.ErrorInvalidState{Reason: fmt.Sprintf("cannot apply %v setting to Longhorn workloads when there are attached volumes", types.SettingNameV2DataEngine)}
}

// Check if there is enough hugepages-2Mi capacity for all nodes
Expand Down Expand Up @@ -463,6 +457,19 @@ func (s *DataStore) AreAllVolumesDetached() (bool, error) {
return true, nil
}

func (s *DataStore) AreAllVolumesDetachedState() (bool, error) {
list, err := s.ListVolumesRO()
if err != nil {
return false, errors.Wrapf(err, "failed to list volumes")
}
for _, v := range list {
if v.Status.State != longhorn.VolumeStateDetached {
return false, nil
}
}
return true, nil
}

func (s *DataStore) getSettingRO(name string) (*longhorn.Setting, error) {
return s.settingLister.Settings(s.namespace).Get(name)
}
Expand Down
13 changes: 13 additions & 0 deletions types/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,14 @@ func (e *NotFoundError) Error() string {
return fmt.Sprintf("cannot find %v", e.Name)
}

type ErrorInvalidState struct {
Reason string
}

func (e *ErrorInvalidState) Error() string {
return fmt.Sprintf("current state prevents this: %v", e.Reason)
}

const (
engineSuffix = "-e"
replicaSuffix = "-r"
Expand Down Expand Up @@ -706,6 +714,11 @@ func ErrorAlreadyExists(err error) bool {
return strings.Contains(err.Error(), "already exists")
}

func ErrorIsInvalidState(err error) bool {
var dummy *ErrorInvalidState
return errors.As(err, &dummy)
}

func ValidateReplicaCount(count int) error {
if count < 1 || count > 20 {
return fmt.Errorf("replica count value must between 1 to 20")
Expand Down

0 comments on commit 380c0c3

Please sign in to comment.