-
Notifications
You must be signed in to change notification settings - Fork 581
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
feat: udevd rules controller #7164
base: main
Are you sure you want to change the base?
Conversation
New resources: ❯ talosctl -n 10.5.0.3 get udevrules
WARNING: 10.5.0.3: server version 1.4.0-alpha.4-46-g7554a3557-dirty is older than client version 1.4.1
NODE NAMESPACE TYPE ID VERSION
10.5.0.3 files UdevRule 1sr89jxq 1
❯ talosctl -n 10.5.0.3 get udevrules -o yaml
WARNING: 10.5.0.3: server version 1.4.0-alpha.4-46-g7554a3557-dirty is older than client version 1.4.1
node: 10.5.0.3
metadata:
namespace: files
type: UdevRules.files.talos.dev
id: 1sr89jxq
version: 1
owner: files.UdevRuleController
phase: running
created: 2023-05-01T18:50:14Z
updated: 2023-05-01T18:50:14Z
spec:
rule: SUBSYSTEM=="block", KERNEL=="vdb*", SYMLINK+="mygreathdd%n"
❯ talosctl -n 10.5.0.3 get udevrulestatus
WARNING: 10.5.0.3: server version 1.4.0-alpha.4-46-g7554a3557-dirty is older than client version 1.4.1
NODE NAMESPACE TYPE ID VERSION
10.5.0.3 files UdevRuleStatus udev 1
❯ talosctl -n 10.5.0.3 get udevrulestatus -o yaml
WARNING: 10.5.0.3: server version 1.4.0-alpha.4-46-g7554a3557-dirty is older than client version 1.4.1
node: 10.5.0.3
metadata:
namespace: files
type: UdevRuleStatuses.files.talos.dev
id: udev
version: 1
owner: files.UdevRuleFileController
phase: running
created: 2023-05-01T18:50:13Z
updated: 2023-05-01T18:50:13Z
spec:
active: true
❯ talosctl -n 10.5.0.3 read /usr/etc/udev/rules.d/99-talos.rules
SUBSYSTEM=="block", KERNEL=="vdb*", SYMLINK+="mygreathdd%n" |
@@ -202,6 +202,12 @@ issues: | |||
- path: internal/app/machined/pkg/system/services | |||
linters: | |||
- dupl | |||
- path: internal/app/machined/pkg/controllers/files |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
golangcilint seems really weird now, maybe it;s my cache
@@ -84,29 +85,6 @@ func (suite *CRISeccompProfileSuite) TestReconcileSeccompProfile() { | |||
}) | |||
} | |||
|
|||
suite.AssertWithin(1*time.Second, 100*time.Millisecond, func() error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this was already handled in the test case above
|
||
suite.AssertWithin(1*time.Second, 100*time.Millisecond, func() error { | ||
_, err := ctest.Get[*criseccompresource.SeccompProfile]( | ||
suite, | ||
criseccompresource.NewSeccompProfile("deny.json").Metadata(), | ||
) | ||
if err != nil { | ||
if !state.IsNotFoundError(err) { | ||
return err | ||
if state.IsNotFoundError(err) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this test was always broken 😅
} | ||
} | ||
|
||
func (ctrl *UdevRuleController) generateRuleHash(rule string) string { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is done since we don't take in rule name from machine config
// Run implements controller.Controller interface. | ||
// | ||
// nolint:gocyclo | ||
func (ctrl *UdevRuleFileController) Run(ctx context.Context, r controller.Runtime, logger *zap.Logger) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm confused on how to handle this, currently this behaves like the previous implementation, write all rules to a single file and reload udev. Another option could be have individual file for each rule, but then I am confused on how to prefix file names (priority order based on numeric prefix) and the cleanup process, do we only delete the file the controller created or all other files under the folder (since extensions also bring in udev rules into the same directory)
|
||
if _, err := cmd.RunContext(ctx, "/sbin/udevadm", "trigger", "--type=subsystems", "--action=add"); err != nil { | ||
return err | ||
_, err = st.WatchFor(ctx, resourcefiles.NewUdevRuleStatus("udev").Metadata(), state.WithCondition(func(r resource.Resource) (bool, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure if there's a better way to wait for the rules 🤔
Use a controller for udevd rules. Signed-off-by: Noel Georgi <[email protected]>
2ea45f6
to
6448762
Compare
After discussion with @smira we need to do some planning around the sequencer tasks and probably start with the last task in the sequencer to be converted to a controller. |
This PR is stale because it has been open 45 days with no activity. |
Use a controller for udevd rules.