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.

Sign up for GitHub

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

libct/specconv: reduce init allocations, use global maps, refactor #3281

Merged
merged 6 commits into from
Nov 17, 2021
Merged

libct/specconv: reduce init allocations, use global maps, refactor #3281

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
8fe1e8b
libct/specconv: rm some init allocations
kolyshkin Sep 23, 2021
81586e1
libct/specconv: reuse mountPropagationMapping in parseMountOptions
kolyshkin Nov 12, 2021
2c3792b
libct/specconv: make mountFlags and extensionFlags global
kolyshkin Nov 12, 2021
37c5fd5
libct/specconv: make parseMountOptions return Mount
kolyshkin Nov 12, 2021
6907bec
libct/specconv: remove isSecSuffix regex
kolyshkin Nov 13, 2021
029b73c
libct/spec: replace isValidName regex with a function
kolyshkin Nov 13, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 10 additions & 7 deletions libcontainer/specconv/spec_linux.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -414,8 +414,6 @@ func createLibcontainerMount(cwd string, m specs.Mount) (*configs.Mount, error)
// systemd property name check: latin letters only, at least 3 of them
var isValidName = regexp.MustCompile(`^[a-zA-Z]{3,}$`).MatchString

var isSecSuffix = regexp.MustCompile(`[a-z]Sec$`).MatchString

// Some systemd properties are documented as having "Sec" suffix
// (e.g. TimeoutStopSec) but are expected to have "USec" suffix
// here, so let's provide conversion to improve compatibility.
Expand Down Expand Up @@ -462,11 +460,16 @@ func initSystemdProps(spec *specs.Spec) ([]systemdDbus.Property, error) {
if err != nil {
return nil, fmt.Errorf("Annotation %s=%s value parse error: %w", k, v, err)
}
if isSecSuffix(name) {
name = strings.TrimSuffix(name, "Sec") + "USec"
value, err = convertSecToUSec(value)
if err != nil {
return nil, fmt.Errorf("Annotation %s=%s value parse error: %w", k, v, err)
// Check for Sec suffix.
if trimName := strings.TrimSuffix(name, "Sec"); len(trimName) < len(name) {
// Check for a lowercase ascii a-z just before Sec.
if ch := trimName[len(trimName)-1]; ch >= 'a' && ch <= 'z' {
// Convert from Sec to USec.
name = trimName + "USec"
value, err = convertSecToUSec(value)
if err != nil {
return nil, fmt.Errorf("Annotation %s=%s value parse error: %w", k, v, err)
Copy link
Member

Choose a reason for hiding this comment

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

Not something new, so probably better to fix separately, but this should've been lowercase

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are you aware of any linter for such things? Would be nice to have

Copy link
Member

Choose a reason for hiding this comment

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

I expect the revive linter to be warning about it; 2cca114

Copy link
Member

Choose a reason for hiding this comment

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

(that's a commit from #2998)

}
}
}
sp = append(sp, systemdDbus.Property{Name: name, Value: value})
Expand Down
5 changes: 5 additions & 0 deletions libcontainer/specconv/spec_linux_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -697,6 +697,11 @@ func TestInitSystemdProps(t *testing.T) {
in: inT{"org.systemd.property.TimeoutStopSec", "'covfefe'"},
exp: expT{true, "", ""},
},
{
desc: "convert USec to Sec (bad variable name, no conversion)",
in: inT{"org.systemd.property.FOOSec", "123"},
exp: expT{false, "FOOSec", 123},
},
{
in: inT{"org.systemd.property.CollectMode", "'inactive-or-failed'"},
exp: expT{false, "CollectMode", "inactive-or-failed"},
Expand Down