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

RSDK-8719: - use maintenance config in RDK #4396

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

Kschappacher
Copy link
Member

@Kschappacher Kschappacher commented Sep 25, 2024

Feature

  • allow users to configure a maintenance sensor on a machine
  • This sensor will inform reconfigure about whether it is safe to reconfigure

@viambot viambot added the safe to test This pull request is marked safe to test from a trusted zone label Sep 25, 2024
@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Sep 25, 2024
@Kschappacher Kschappacher changed the title first pass RSDK-8719 - use maintenance config in RDK Sep 27, 2024
@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Sep 27, 2024
@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Sep 27, 2024
@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Sep 27, 2024
Comment on lines +1449 to +1451
if newConfig.MaintenanceConfig == nil {
return true, errors.New("maintenanceConfig undefined. Using default reconfigure")
}
Copy link
Member Author

Choose a reason for hiding this comment

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

move this up to where this is called and don't alert users if this does not exist

robot/impl/local_robot.go Show resolved Hide resolved
robot/impl/local_robot.go Show resolved Hide resolved
return true, errors.Errorf("maintenanceAllowedKey %s is not a bool value", maintenanceAllowedKey)
}

r.logger.Info("maintenanceAllowedKey found canReconfigure set to %t", canReconfigure)
Copy link
Member Author

Choose a reason for hiding this comment

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

maintenanceAllowedKey found on readings from maintenance sensor explicitly say skipping reconfiguration or starting reconfiguration

robot/impl/local_robot.go Show resolved Hide resolved
errorMessage string
}{
{
canReconfigure: true,
Copy link
Member Author

Choose a reason for hiding this comment

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

add testname field

sensor: newSensor(),
errorMessage: "error getting MaintenanceAllowedKey UnknownKey from sensor reading",
},
}
Copy link
Member Author

Choose a reason for hiding this comment

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

add test for non boolean return

}

func (r *localRobot) checkMaintenanceSensorReadings(maintenanceAllowedKey string, sensor resource.Sensor) (bool, error) {
readings, err := sensor.Readings(context.Background(), map[string]interface{}{})
Copy link
Member Author

Choose a reason for hiding this comment

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

add context timeout here and add test for the timeout

Copy link
Member Author

Choose a reason for hiding this comment

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

5 seconds

@10zingpd 10zingpd changed the title RSDK-8719 - use maintenance config in RDK RSDK-8719: - use maintenance config in RDK Oct 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
safe to test This pull request is marked safe to test from a trusted zone
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants