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

Improve some handling around packages #323

Merged
merged 5 commits into from
Sep 25, 2023
Merged

Improve some handling around packages #323

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8001d7c
Improve some handling around packages
stvnrhodes Sep 14, 2023
7c8f735
Remove unused var
stvnrhodes Sep 15, 2023
399494e
Add error handling
stvnrhodes Sep 15, 2023
492924d
Add some more help text
stvnrhodes Sep 22, 2023
f76b9ab
Tweak error message
stvnrhodes Sep 22, 2023
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
66 changes: 45 additions & 21 deletions services/packages/client/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,8 @@ func (*installCmd) Synopsis() string { return "Install a new package" }
func (*installCmd) Usage() string {
return `install [--package_system=P] --name=X --version=Y [--disablerepo=A] [--repo|enablerepo=Z]:
Install a new package on the remote machine.

Version format is NEVRA without the name, like --name bash --new_version 0:4.2.46-34.el7.x86_64
`
}

Expand All @@ -118,7 +120,8 @@ func (i *installCmd) Execute(ctx context.Context, f *flag.FlagSet, args ...inter

if f.NArg() != 0 {
fmt.Fprintln(os.Stderr, "All options are set via flags")
return subcommands.ExitFailure
subcommands.DefaultCommander.ExplainCommand(os.Stderr, i)
return subcommands.ExitUsageError
}
if i.name == "" || i.version == "" {
fmt.Fprintln(os.Stderr, "Both --name and --version must be filled in")
Expand Down Expand Up @@ -186,7 +189,8 @@ func (r *removeCmd) Execute(ctx context.Context, f *flag.FlagSet, args ...interf

if f.NArg() != 0 {
fmt.Fprintln(os.Stderr, "All options are set via flags")
return subcommands.ExitFailure
subcommands.DefaultCommander.ExplainCommand(os.Stderr, r)
return subcommands.ExitUsageError
}
if r.name == "" || r.version == "" {
fmt.Fprintln(os.Stderr, "Both --name and --version must be filled in")
Expand Down Expand Up @@ -237,6 +241,8 @@ func (*updateCmd) Synopsis() string { return "Update an existing package" }
func (*updateCmd) Usage() string {
return `update [--package_system=P] --name=X --old_version=Y --new_version=Z [--disablerepo=B] [--repo|enablerepo=A]:
Update a package on the remote machine. The package must already be installed at a known version.

Version format is NEVRA without the name, like -name bash -old_version 0:4.2.46-34.el7.x86_64 -new_version 0:4.2.46-35.el7_9.x86_64
`
}

Expand All @@ -253,7 +259,8 @@ func (u *updateCmd) SetFlags(f *flag.FlagSet) {
func (u *updateCmd) Execute(ctx context.Context, f *flag.FlagSet, args ...interface{}) subcommands.ExitStatus {
if f.NArg() != 0 {
fmt.Fprintln(os.Stderr, "All options are set via flags")
return subcommands.ExitFailure
subcommands.DefaultCommander.ExplainCommand(os.Stderr, u)
return subcommands.ExitUsageError
}
if u.name == "" || u.oldVersion == "" || u.newVersion == "" {
fmt.Fprintln(os.Stderr, "--name, --old_version and --new_version must be supplied")
Expand Down Expand Up @@ -301,24 +308,27 @@ func (u *updateCmd) Execute(ctx context.Context, f *flag.FlagSet, args ...interf

type listCmd struct {
packageSystem string
nevra bool
}

func (*listCmd) Name() string { return "list" }
func (*listCmd) Synopsis() string { return "List installed packages" }
func (*listCmd) Usage() string {
return `list [--package_system=P]:
return `list [--package_system=P] [--nevra]:
List the installed packages on the remote machine.
`
}

func (l *listCmd) SetFlags(f *flag.FlagSet) {
f.StringVar(&l.packageSystem, "package-system", "YUM", fmt.Sprintf("Package system to use(one of: [%s])", strings.Join(shortPackageSystemNames(), ",")))
f.BoolVar(&l.nevra, "nevra", false, "For YUM, print output in NEVRA format instead of trying to imitate `yum list-installed`")
}

func (l *listCmd) Execute(ctx context.Context, f *flag.FlagSet, args ...interface{}) subcommands.ExitStatus {
if f.NArg() != 0 {
fmt.Fprintln(os.Stderr, "All options are set via flags")
return subcommands.ExitFailure
subcommands.DefaultCommander.ExplainCommand(os.Stderr, l)
return subcommands.ExitUsageError
}
ps, err := flagToType(l.packageSystem)
if err != nil {
Expand Down Expand Up @@ -347,21 +357,32 @@ func (l *listCmd) Execute(ctx context.Context, f *flag.FlagSet, args ...interfac
retCode = subcommands.ExitFailure
continue
}
fmt.Fprint(state.Out[r.Index], "Installed Packages\n")
for _, pkg := range r.Resp.Packages {
// Print the package name.arch, [epoch]:version-release and repo with some reasonable spacing.
na := pkg.Name
if pkg.Architecture != "" {
na = na + "." + pkg.Architecture
}
evr := pkg.Version
if pkg.Release != "" {
evr = evr + "-" + pkg.Release
if l.nevra {
for _, pkg := range r.Resp.Packages {
ne := pkg.Name
if pkg.Epoch != 0 {
ne = fmt.Sprintf("%s-%d", pkg.Name, pkg.Epoch)
}
nevra := fmt.Sprintf("%v:%v-%v.%v", ne, pkg.Version, pkg.Release, pkg.Architecture)
fmt.Fprintln(state.Out[r.Index], nevra)
}
if pkg.Epoch != 0 {
evr = fmt.Sprintf("%d:%s", pkg.Epoch, evr)
} else {
fmt.Fprint(state.Out[r.Index], "Installed Packages\n")
for _, pkg := range r.Resp.Packages {
// Print the package name.arch, [epoch]:version-release and repo with some reasonable spacing.
na := pkg.Name
if pkg.Architecture != "" {
na = na + "." + pkg.Architecture
}
evr := pkg.Version
if pkg.Release != "" {
evr = evr + "-" + pkg.Release
}
if pkg.Epoch != 0 {
evr = fmt.Sprintf("%d:%s", pkg.Epoch, evr)
}
fmt.Fprintf(state.Out[r.Index], "%40s %16s %32s\n", na, evr, pkg.Repo)
}
fmt.Fprintf(state.Out[r.Index], "%40s %16s %32s\n", na, evr, pkg.Repo)
}
}
return retCode
Expand Down Expand Up @@ -393,7 +414,8 @@ func (l *searchCmd) SetFlags(f *flag.FlagSet) {
func (l *searchCmd) Execute(ctx context.Context, f *flag.FlagSet, args ...interface{}) subcommands.ExitStatus {
if f.NArg() != 0 {
fmt.Fprintln(os.Stderr, "All options are set via flags")
return subcommands.ExitFailure
subcommands.DefaultCommander.ExplainCommand(os.Stderr, l)
return subcommands.ExitUsageError
}
if l.name == "" {
fmt.Fprintln(os.Stderr, "--name must be supplied")
Expand Down Expand Up @@ -481,7 +503,8 @@ func (r *repoListCmd) SetFlags(f *flag.FlagSet) {
func (r *repoListCmd) Execute(ctx context.Context, f *flag.FlagSet, args ...interface{}) subcommands.ExitStatus {
if f.NArg() != 0 {
fmt.Fprintln(os.Stderr, "All options are set via flags")
return subcommands.ExitFailure
subcommands.DefaultCommander.ExplainCommand(os.Stderr, r)
return subcommands.ExitUsageError
}
ps, err := flagToType(r.packageSystem)
if err != nil {
Expand Down Expand Up @@ -562,7 +585,8 @@ func (r *cleanupCmd) SetFlags(f *flag.FlagSet) {
func (r *cleanupCmd) Execute(ctx context.Context, f *flag.FlagSet, args ...interface{}) subcommands.ExitStatus {
if f.NArg() != 0 {
fmt.Fprintln(os.Stderr, "All options are set via flags")
return subcommands.ExitFailure
subcommands.DefaultCommander.ExplainCommand(os.Stderr, r)
return subcommands.ExitUsageError
}
ps, err := flagToType(r.packageSystem)
if err != nil {
Expand Down
106 changes: 102 additions & 4 deletions services/packages/server/packages.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,18 +122,18 @@ var (
return genCmd(p.PackageSystem, removeOpts)
}

generateValidate = func(p *pb.UpdateRequest) ([]string, error) {
generateValidate = func(packageSystem pb.PackageSystem, name, version string) ([]string, error) {
validateOpts := map[pb.PackageSystem][]string{
pb.PackageSystem_PACKAGE_SYSTEM_YUM: {
"list",
"installed",
},
}
out, err := genCmd(p.PackageSystem, validateOpts)
out, err := genCmd(packageSystem, validateOpts)
if err != nil {
return nil, err
}
return addRepoAndPackage(out, p.PackageSystem, p.Name, p.OldVersion, &repoData{}), nil
return addRepoAndPackage(out, packageSystem, name, version, &repoData{}), nil
}

generateUpdate = func(p *pb.UpdateRequest) ([]string, error) {
Expand All @@ -143,6 +143,9 @@ var (
"-y",
},
}
if isOlderVersion(p.NewVersion, p.OldVersion) {
updateOpts[pb.PackageSystem_PACKAGE_SYSTEM_YUM] = []string{"downgrade", "-y"}
}
out, err := genCmd(p.PackageSystem, updateOpts)
if err != nil {
return nil, err
Expand Down Expand Up @@ -225,6 +228,69 @@ var (
}
)

// parseToNumbers parses a string like 14:4.99.3-2.fc38.x86_64 into a slice
// of integers like [14, 4, 99, 3, 2]. Callers should ensure that the input
// is valid.
func parseToNumbers(version string) [5]int {
var out [5]int

// Grab epoch
split := strings.SplitN(version, ":", 2)
if len(split) == 2 {
epoch, err := strconv.Atoi(split[0])
if err != nil {
return out
}
out[0] = epoch
version = split[1]
}

// Grab version numbers
split = strings.SplitN(version, "-", 2)
if len(split) < 2 {
return out
}
for i, n := range strings.SplitN(split[0], ".", 3) {
v, err := strconv.Atoi(n)
if err != nil {
return out
}
out[1+i] = v
}
remainder := split[1]

// Grab revision
split = strings.SplitN(remainder, ".", 2)
if len(split) < 2 {
return out
}
revision, err := strconv.Atoi(split[0])
if err != nil {
return out
}
out[4] = revision

return out
}

// isOlderVersion returns true if "a" is an older version than "b".
// Input should be the same as version in the RPC, like
// 14:4.99.3-2.fc38.x86_64 or 3.43.1-1.centos7.arm64.
// Results are indeterminate if either string isn't a proper version.
func isOlderVersion(a, b string) bool {
parsedA := parseToNumbers(a)
parsedB := parseToNumbers(b)
for i := 0; i < 5; i++ {
if parsedA[i] < parsedB[i] {
return true
}
if parsedA[i] > parsedB[i] {
return false
}
}
return false
}

// server is used to implement the gRPC server
type server struct{}

Expand Down Expand Up @@ -281,6 +347,22 @@ func (s *server) Install(ctx context.Context, req *pb.InstallRequest) (*pb.Insta
}
}

// Make sure we actually installed the package.
postValidateCommand, err := generateValidate(req.PackageSystem, req.Name, req.Version)
if err != nil {
recorder.CounterOrLog(ctx, packagesInstallFailureCounter, 1, attribute.String("reason", "post_validate_err"))
return nil, err
}
validation, err := util.RunCommand(ctx, postValidateCommand[0], postValidateCommand[1:])
if err != nil {
recorder.CounterOrLog(ctx, packagesInstallFailureCounter, 1, attribute.String("reason", "post_validate_err"))
return nil, err
}
if err := validation.Error; validation.ExitCode != 0 || err != nil {
recorder.CounterOrLog(ctx, packagesInstallFailureCounter, 1, attribute.String("reason", "post_validate_err"))
return nil, status.Errorf(codes.Internal, "failed to confirm that package was installed\nstdout:\n%s\nstderr:\n%s\ninstall stdout:\n%s\ninstall stderr:\n%s", util.TrimString(validation.Stdout.String()), util.TrimString(validation.Stderr.String()), util.TrimString(run.Stdout.String()), util.TrimString(run.Stderr.String()))
}

return &pb.InstallReply{
DebugOutput: fmt.Sprintf("%s%s", run.Stdout.String(), run.Stderr.String()),
RebootRequired: isRebootRequired,
Expand Down Expand Up @@ -362,7 +444,7 @@ func (s *server) Update(ctx context.Context, req *pb.UpdateRequest) (*pb.UpdateR
}

// We can generate both commands since errors duplicate here.
validateCommand, valErr := generateValidate(req)
validateCommand, valErr := generateValidate(req.PackageSystem, req.Name, req.OldVersion)
updateCommand, upErr := generateUpdate(req)
if valErr != nil || upErr != nil {
recorder.CounterOrLog(ctx, packagesUpdateFailureCounter, 1, attribute.String("reason", "generate_cmd_err"))
Expand Down Expand Up @@ -391,6 +473,22 @@ func (s *server) Update(ctx context.Context, req *pb.UpdateRequest) (*pb.UpdateR
return nil, status.Errorf(codes.Internal, "error from running - %v\nstdout:\n%s\nstderr:\n%s", err, util.TrimString(run.Stdout.String()), util.TrimString(run.Stderr.String()))
}

// Make sure we actually installed the package.
postValidateCommand, err := generateValidate(req.PackageSystem, req.Name, req.NewVersion)
if err != nil {
recorder.CounterOrLog(ctx, packagesUpdateFailureCounter, 1, attribute.String("reason", "post_validate_err"))
return nil, err
}
validation, err := util.RunCommand(ctx, postValidateCommand[0], postValidateCommand[1:])
if err != nil {
recorder.CounterOrLog(ctx, packagesUpdateFailureCounter, 1, attribute.String("reason", "post_validate_err"))
return nil, err
}
if err := validation.Error; validation.ExitCode != 0 || err != nil {
recorder.CounterOrLog(ctx, packagesUpdateFailureCounter, 1, attribute.String("reason", "post_validate_err"))
return nil, status.Errorf(codes.Internal, "failed to confirm that package was installed\nstdout:\n%s\nstderr:\n%s\ninstall stdout:\n%s\ninstall stderr:\n%s", util.TrimString(validation.Stdout.String()), util.TrimString(validation.Stderr.String()), util.TrimString(run.Stdout.String()), util.TrimString(run.Stderr.String()))
}

return &pb.UpdateReply{
DebugOutput: fmt.Sprintf("%s%s", run.Stdout.String(), run.Stderr.String()),
}, nil
Expand Down
Loading