Skip to content

Commit

Permalink
Improve some handling around packages (#323)
Browse files Browse the repository at this point in the history 
* Improve some handling around packages

- Add a mode for `packages list` that gives a format closer to what we use elsewhere in sansshell
- Validate that the package we wanted to install was actually installed in both `InstallPackage` and `UpdatePackage`
- Print out flags on some usage errors
- Automatically use `yum downgrade` instead of `update-to` when `UpdatePackage` is called to update a package to an older version

Interestingly, the behavior of `yum install-nevra` semms to differ between centos7 and fedora. On centos7 I see it refusing to downgrade and on fedora I see it installing the exact specified version.

* Remove unused var

* Add error handling

* Add some more help text

* Tweak error message
  • Loading branch information
@stvnrhodes
stvnrhodes authored Sep 25, 2023
1 parent 3a057e1 commit c872598
Show file tree
Hide file tree
Showing 3 changed files with 202 additions and 33 deletions.
66 changes: 45 additions & 21 deletions services/packages/client/client.go
View file
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
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

0 comments on commit c872598

Please sign in to comment.