libcontainer/intelrdt: modify the incorrect file mode

[KentaTada]

This file mode is unintentional apparently.

Signed-off-by: Kenta Tada [email protected]

[thaJeztah]

LGTM

did the linter catch this? (if not, we may have to update the configuration, as I think it should be able to catch this)

[kolyshkin]

Yes, it's a linter error, I've been meaning to fix this but didn't have time yet

[kolyshkin]

Just for the sake of sanity, I'd prefer 0644 here. Or, using the latest and greatest golang, 0o644.

[KentaTada]

I wanted to respect the original commit in this pull request.
But if maintainers don't mind it, I'll change from 0777 to 0o644 in this pull request.

[KentaTada]

@kolyshkin I changed the file mode. Thanks!

[kolyshkin]

a nit

[KentaTada]

did the linter catch this? (if not, we may have to update the configuration, as I think it should be able to catch this)

Yes.
I also tested runc using linter and I found it.

[thaJeztah]

Looks like the linter on PR's is perhaps only checking the changed lines; I saw Akihiro's comment on #2618 (comment), and the linter picked up the problem in https://github.com/opencontainers/runc/runs/1190924966

Looks like that output is a good list of things to work on;

Error: `userFromOS` is unused (deadcode)
Error: `groupFromOS` is unused (deadcode)
Error: `loadConfig` is unused (deadcode)
Error: `removePath` is unused (deadcode)
Error: `wildcard` is unused (deadcode)
Error: Error return value of `os.Mkdir` is not checked (errcheck)
Error: Error return value of `os.Symlink` is not checked (errcheck)
Error: Error return value of `console.ClearONLCR` is not checked (errcheck)
Error: Error return value of `io.Copy` is not checked (errcheck)
Error: Error return value of `io.Copy` is not checked (errcheck)
Error: Error return value of `io.Copy` is not checked (errcheck)
Error: Error return value of `cmd.Process.Kill` is not checked (errcheck)
Error: Error return value of `cmd.Wait` is not checked (errcheck)
Error: Error return value of `fHook.Run` is not checked (errcheck)
Error: Error return value of `join` is not checked (errcheck)
Error: Error return value of `os.Symlink` is not checked (errcheck)
Error: Error return value of `dbusConnection.ResetFailedUnit` is not checked (errcheck)
Error: Error return value of `dbusConnection.ResetFailedUnit` is not checked (errcheck)
Error: Error return value of `unix.Unmount` is not checked (errcheck)
Error: Error return value of `criuClientCon.CloseWrite` is not checked (errcheck)
Error: Error return value of `Cgroupfs` is not checked (errcheck)
Error: Error return value of `unix.Unmount` is not checked (errcheck)
Error: Error return value of `m.Freeze` is not checked (errcheck)
Error: Error return value of `p.wait` is not checked (errcheck)
Error: Error return value of `p.cmd.Wait` is not checked (errcheck)
Error: Error return value of `p.cmd.Wait` is not checked (errcheck)
Error: Error return value of `p.cmd.Wait` is not checked (errcheck)
Error: Error return value of `p.manager.Destroy` is not checked (errcheck)
Error: Error return value of `p.intelRdtManager.Destroy` is not checked (errcheck)
Error: Error return value of `p.wait` is not checked (errcheck)
Error: Error return value of `signalAllProcesses` is not checked (errcheck)
Error: Error return value of `unix.Unmount` is not checked (errcheck)
Error: Error return value of `cleanupTmp` is not checked (errcheck)
Error: Error return value of `selinux.SetKeyLabel` is not checked (errcheck)
Error: Error return value of `selinux.SetExecLabel` is not checked (errcheck)
Error: Error return value of `selinux.SetKeyLabel` is not checked (errcheck)
Error: Error return value of `selinux.SetExecLabel` is not checked (errcheck)
Error: Error return value of `i.c.initProcess.signal` is not checked (errcheck)
Error: Error return value of `p.Wait` is not checked (errcheck)
Error: Error return value of `container.Destroy` is not checked (errcheck)
Error: Error return value of `showFile` is not checked (errcheck)
Error: Error return value of `showFile` is not checked (errcheck)
Error: Error return value of `showFile` is not checked (errcheck)
Error: Error return value of `container.Destroy` is not checked (errcheck)
Error: Error return value of `container.Destroy` is not checked (errcheck)
Error: Error return value of `unmountOp` is not checked (errcheck)
Error: Error return value of `unmountOp` is not checked (errcheck)
Error: Error return value of `unmountOp` is not checked (errcheck)
Error: Error return value of `container1.Destroy` is not checked (errcheck)
Error: Error return value of `container2.Destroy` is not checked (errcheck)
Error: Error return value of `container1.Destroy` is not checked (errcheck)
Error: Error return value of `container2.Destroy` is not checked (errcheck)
Error: Error return value of `console.ClearONLCR` is not checked (errcheck)
Error: Error return value of `h.Write` is not checked (errcheck)
Error: Error return value of `client.Write` is not checked (errcheck)
Error: ineffectual assignment to `paths` (ineffassign)
Error: `si_code` is unused (structcheck)
Error: `pad` is unused (structcheck)
Error: `si_signo` is unused (structcheck)
Error: `si_errno` is unused (structcheck)
Error: S1002: should omit comparison to bool constant, can be simplified to `ok` (gosimple)
Error: S1002: should omit comparison to bool constant, can be simplified to `ok` (gosimple)
Error: S1002: should omit comparison to bool constant, can be simplified to `ok` (gosimple)
Error: S1002: should omit comparison to bool constant, can be simplified to `!symLink` (gosimple)
Error: S1032: should use sort.Ints(...) instead of sort.Sort(sort.IntSlice(...)) (gosimple)
Error: S1032: should use sort.Ints(...) instead of sort.Sort(sort.IntSlice(...)) (gosimple)
Error: S1023: redundant `return` statement (gosimple)
Error: S1023: redundant `return` statement (gosimple)
Error: S1017: should replace this `if` statement with an unconditional `strings.TrimPrefix` (gosimple)
Error: S1017: should replace this `if` statement with an unconditional `strings.TrimPrefix` (gosimple)
Error: S1006: should use for {} instead of for true {} (gosimple)
Error: S1008: should use 'return err == nil' instead of 'if err != nil { return false }; return true' (gosimple)
Error: S1021: should merge variable declaration with assignment on next line (gosimple)
Error: S1021: should merge variable declaration with assignment on next line (gosimple)
Error: S1030: should use stdout.String() instead of string(stdout.Bytes()) (gosimple)
Error: S1030: should use stdout.String() instead of string(stdout.Bytes()) (gosimple)
Error: S1030: should use stdout2.String() instead of string(stdout2.Bytes()) (gosimple)
Error: S1030: should use stdout.String() instead of string(stdout.Bytes()) (gosimple)
Error: S1030: should use stdout2.String() instead of string(stdout2.Bytes()) (gosimple)
Error: SA4006: this value of `process` is never used (staticcheck)
Error: SA4000: identical expressions on the left and right side of the '-' operator (staticcheck)
Error: SA5001: should check returned error before deferring file.Close() (staticcheck)
Error: SA9002: file mode '777' evaluates to 01411; did you mean '0777'? (staticcheck)
Error: SA1019: prog.Attach is deprecated: use link.RawAttachProgram instead.  (staticcheck)
Error: SA1019: prog.Detach is deprecated: use link.RawDetachProgram instead.  (staticcheck)
Error: SA1019: package github.com/golang/protobuf/proto is deprecated: Use the "google.golang.org/protobuf/proto" package instead.  (staticcheck)
Error: func `(*linuxContainer).deleteState` is unused (unused)

Error: issues found

Some notes there;

• I'm not sure it the logs output all errors, or a limited list; usually, golang-ci-lint is configured to limit some issues if they occur too many times
• Depending on the above, may consider disabling errcheck, or configure a rule/regex for error-handling we think is acceptable to ignore

[KentaTada]

I'm not sure it the logs output all errors, or a limited list; usually, golang-ci-lint is configured to limit some issues if they occur too many times

Yes.

For example,

Error: userFromOS is unused (deadcode)
Error: groupFromOS is unused (deadcode)

I think those functions are actually used for windows environment. Doesn't the current CI script recognize windows environment?

[thaJeztah]

I think those functions are actually used for windows environment. Doesn't the current CI script recognize windows environment?

Not sure, it may be only checking for the platform it runs on. IIRC, there's also an option to specify which build tags to use when linting. (so multiple combinations of build-tags could be needed to lint everything).

These should also be ignored likely (but could potentially be candidates for inclusion in golang.org/x/sys);

Error: `si_code` is unused (structcheck)
Error: `pad` is unused (structcheck)
Error: `si_signo` is unused (structcheck)
Error: `si_errno` is unused (structcheck)

[thaJeztah]

Let me open a new ticket for tracking linting issues

[KentaTada]

Let me open a new ticket for tracking linting issues

Thanks!

[thaJeztah]

Opened #2627 as a quick first draft 👍

[thaJeztah]

For consistency, would be good if we did a look for all 0755, 0644, 0777 and change to the new 0o... format (possibly look if perms are correct as well, e.g. in some cases 0700 or 0600 could be correct if we expect files to be exclusively used by runc?)