overlay: add GCP udev rules #160
Conversation
|
Worth noting is that the BZ references one other udev rule ( |
|
These rules aren't conditionalized for GCP, so they'd run on every platform. We should limit them to GCP. Here's how Container Linux does it. |
|
Additionally, this would hijack filepaths that belong to other packages (In Fedora, that would be google-compute-engine-tools). Have we considered splitting the binary package out instead of duplicating them? That would also help with maintenance/updates/ownership. |
Doh! I didn't bother to check; just assumed that they would behave like the Azure rules in #159. I'll rework this.
Similar to what you've proposed for the Azure rules? |
That doesn't seem like a real problem to me; we're not supporting installing the agent on FCOS right? That said, it would be cleaner to have them come from a |
|
Oh, definitely not something to stall this on, but more like whether a plan was in place for longer term (similar to Azure, yes). |
|
I'd like to keep the existing rules as intact as possible, so would it be reasonable to scope them with Related: if the rules are changed as such, how does the copyright stanza have to change, if at all? |
Yep, understood. We're basically making the rules up as we go along, so to speak. |
|
@miabbott I'm not super-comfortable with the way the rules assign to We could add a 2019 Red Hat copyright statement, but AIUI there's not a strong need to do so. |
It'd be safe to propose a patch upstream for this too, right? |
miabbott
force-pushed
the
add_gcp_udev_rules
branch
from
d061d15 to
fd66ae7
Compare
|
Pushed a commit with additional scoping ⬆️ Thanks for the feedback so far! |
The downstream BZ#1748638[0] requests the addition of udev rules for nodes in GCP to support the dynamic provisioning of storage. The rules live in the `GoogleCloudPlatform/compute-image-packages` repo[1] and were most recently updated in Dec 2018. This commit is mostly a copy of those rules with additional scoping to only run in the GCP environment. [0] https://bugzilla.redhat.com/show_bug.cgi?id=1748638 [1] https://github.com/GoogleCloudPlatform/compute-image-packages/tree/master/packages/google-compute-engine/src/lib/udev/rules.d
miabbott
force-pushed
the
add_gcp_udev_rules
branch
from
fd66ae7 to
3f62b20
Compare
|
Rebased to see if CI will be happy 🤞 |
|
One thing I wanted to quickly verify is the vendor, and yep, LGTM: |
| # | ||
| # When a disk is removed, unmount any remaining attached volumes. | ||
|
|
||
| ACTION=="remove", SUBSYSTEM=="block", KERNEL=="sd*|vd*", ENV{ID_VENDOR}=="Google", RUN+="/bin/sh -c '/bin/umount -fl /dev/$name && /usr/bin/logger -p daemon.warn -s WARNING: hot-removed /dev/$name that was still mounted, data may have been corrupted'" |
There was a problem hiding this comment.
I'm not sure about this rule. It means that we're behaving differently on GCP than on other platforms when devices are yanked from the system.
There was a problem hiding this comment.
Yeah, in the end I don't think it's really specific to GCE, on at least AWS too one can force detach with the API.
And AIUI this occurs when e.g. higher level code like Kubernetes Persistent Volume management cleans up resources before it unmounts it on the OS.
There was a problem hiding this comment.
Could we just drop this file?
There was a problem hiding this comment.
The Bugzilla that spawned this doesn't say exactly what went wrong...but presumably it was related to the naming, and not this hack to do the unmount. So we probably could drop it...
There was a problem hiding this comment.
I pinged the reporter in the BZ about the removal rules. We can carry them in RHCOS if needed, but I agree we could probably remove them for FCOS.
This reverts a change in a section heading, which broke an incoming link from coreos#528.
The downstream BZ#1748638[0] requests the addition of udev rules for
nodes in GCP to support the dynamic provisioning of storage. The
rules live in the
GoogleCloudPlatform/compute-image-packagesrepo[1]and were most recently updated in Dec 2018. This commit is a copy of
the rules from the repo.
[0] https://bugzilla.redhat.com/show_bug.cgi?id=1748638
[1] https://github.com/GoogleCloudPlatform/compute-image-packages/tree/master/packages/google-compute-engine/src/lib/udev/rules.d