Skip to content

Commit

Permalink
feat(opm): configure diff to not include dependency bundles with --sk…
Browse files Browse the repository at this point in the history
…ip-deps (#753)

Signed-off-by: Eric Stroczynski <ericstroczynski@gmail.com>
  • Loading branch information
Eric Stroczynski committed Aug 19, 2021
1 parent d487288 commit 7fba6ad
Show file tree
Hide file tree
Showing 4 changed files with 163 additions and 26 deletions.
47 changes: 29 additions & 18 deletions cmd/opm/alpha/diff/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,9 @@ const (
)

type diff struct {
oldRefs []string
newRefs []string
oldRefs []string
newRefs []string
skipDeps bool

output string
caFile string
Expand Down Expand Up @@ -59,26 +60,33 @@ in which case they are not included in the diff, or a new ref, in which
case they are included. Dependencies provided by some catalog unknown to
'opm alpha diff' will not cause the command to error, but an error will occur
if that catalog is not serving these dependencies at runtime.
Dependency inclusion can be turned off with --no-deps, although this is not recommended
unless you are certain some in-cluster catalog satisfies all dependencies.
NOTE: for now, if any dependency exists, the entire dependency's package is added to the diff.
In the future, these packages will be pruned such that only the latest dependencies
satisfying a package version range or GVK, and their upgrade graph(s) to their latest
channel head(s), are included in the diff.
`),
Example: templates.Examples(`
# Diff a catalog at some old state and latest state into a declarative config index.
mkdir -p catalog-index
opm alpha diff registry.org/my-catalog:abc123 registry.org/my-catalog:def456 -o yaml > ./my-catalog-index/index.yaml
# Create a directory for your declarative config diff.
mkdir -p my-catalog-index
# Build and push this index into an index image.
opm alpha generate dockerfile ./my-catalog-index
docker build -t registry.org/my-catalog:latest-abc123-def456 -f index.Dockerfile .
docker push registry.org/my-catalog:latest-abc123-def456
# THEN:
# Create a new catalog from a diff between an old and the latest
# state of a catalog as a declarative config index.
opm alpha diff registry.org/my-catalog:abc123 registry.org/my-catalog:def456 -o yaml > ./my-catalog-index/index.yaml
# Create a new catalog from the heads of an existing catalog, then build and push the image like above.
# OR:
# Create a new catalog from the heads of an existing catalog.
opm alpha diff registry.org/my-catalog:def456 -o yaml > my-catalog-index/index.yaml
docker build -t registry.org/my-catalog:headsonly-def456 -f index.Dockerfile .
docker push registry.org/my-catalog:headsonly-def456
# FINALLY:
# Build an index image containing the diff-ed declarative config,
# then tag and push it.
opm alpha generate dockerfile ./my-catalog-index
docker build -t registry.org/my-catalog:diff-latest -f index.Dockerfile .
docker push registry.org/my-catalog:diff-latest
`),
Args: cobra.RangeArgs(1, 2),
PreRunE: func(cmd *cobra.Command, args []string) error {
Expand All @@ -91,6 +99,8 @@ docker push registry.org/my-catalog:headsonly-def456
RunE: a.addFunc,
}

cmd.Flags().BoolVar(&a.skipDeps, "skip-deps", false, "do not include bundle dependencies in the output catalog")

cmd.Flags().StringVarP(&a.output, "output", "o", "yaml", "Output format (json|yaml)")
cmd.Flags().StringVarP(&a.caFile, "ca-file", "", "", "the root Certificates to use with this command")

Expand All @@ -103,7 +113,7 @@ func (a *diff) addFunc(cmd *cobra.Command, args []string) error {

skipTLS, err := cmd.Flags().GetBool("skip-tls")
if err != nil {
panic(err)
logrus.Panic(err)
}

var write func(declcfg.DeclarativeConfig, io.Writer) error
Expand Down Expand Up @@ -134,10 +144,11 @@ func (a *diff) addFunc(cmd *cobra.Command, args []string) error {
defer cancel()

diff := action.Diff{
Registry: reg,
OldRefs: a.oldRefs,
NewRefs: a.newRefs,
Logger: a.logger,
Registry: reg,
OldRefs: a.oldRefs,
NewRefs: a.newRefs,
SkipDependencies: a.skipDeps,
Logger: a.logger,
}
cfg, err := diff.Run(ctx)
if err != nil {
Expand All @@ -159,7 +170,7 @@ func (a *diff) parseArgs(args []string) {
case 2:
old, new = args[0], args[1]
default:
panic("should never be here, CLI must enforce arg size")
logrus.Panic("should never be here, CLI must enforce arg size")
}
if old != "" {
a.oldRefs = strings.Split(old, ",")
Expand Down
9 changes: 8 additions & 1 deletion internal/action/diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ type Diff struct {

OldRefs []string
NewRefs []string
// SkipDependencies directs Run() to not include dependencies
// of bundles included in the diff if true.
SkipDependencies bool

Logger *logrus.Entry
}
Expand Down Expand Up @@ -59,7 +62,11 @@ func (a Diff) Run(ctx context.Context) (*declcfg.DeclarativeConfig, error) {
return nil, fmt.Errorf("error converting new declarative config to model: %v", err)
}

diffModel, err := declcfg.Diff(oldModel, newModel)
g := &declcfg.DiffGenerator{
Logger: a.Logger,
SkipDependencies: a.SkipDependencies,
}
diffModel, err := g.Run(oldModel, newModel)
if err != nil {
return nil, fmt.Errorf("error generating diff: %v", err)
}
Expand Down
34 changes: 29 additions & 5 deletions internal/declcfg/diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,41 @@ package declcfg
import (
"reflect"
"sort"
"sync"

"github.com/blang/semver"
"github.com/mitchellh/hashstructure/v2"
"github.com/sirupsen/logrus"

"github.com/operator-framework/operator-registry/internal/model"
"github.com/operator-framework/operator-registry/internal/property"
)

// Diff returns a Model containing everything in newModel not in oldModel,
// DiffGenerator configures how diffs are created via Run().
type DiffGenerator struct {
Logger *logrus.Entry

// SkipDependencies directs Run() to not include dependencies
// of bundles included in the diff if true.
SkipDependencies bool

initOnce sync.Once
}

func (g *DiffGenerator) init() {
g.initOnce.Do(func() {
if g.Logger == nil {
g.Logger = &logrus.Entry{}
}
})
}

// Run returns a Model containing everything in newModel not in oldModel,
// and all bundles that exist in oldModel but are different in newModel.
// If oldModel is empty, only channel heads in newModel's packages are
// added to the output Model. All dependencies not in oldModel are also added.
func Diff(oldModel, newModel model.Model) (model.Model, error) {
func (g *DiffGenerator) Run(oldModel, newModel model.Model) (model.Model, error) {
g.init()

// TODO(estroz): loading both oldModel and newModel into memory may
// exceed process/hardware limits. Instead, store models on-disk then
Expand Down Expand Up @@ -71,9 +93,11 @@ func Diff(oldModel, newModel model.Model) (model.Model, error) {
}
}

// Add dependencies to outputModel not already present in oldModel.
if err := addAllDependencies(newModel, oldModel, outputModel); err != nil {
return nil, err
if !g.SkipDependencies {
// Add dependencies to outputModel not already present in oldModel.
if err := addAllDependencies(newModel, oldModel, outputModel); err != nil {
return nil, err
}
}

// Default channel may not have been copied, so set it to the new default channel here.
Expand Down
Loading

0 comments on commit 7fba6ad

Please sign in to comment.