Skip to content

Commit

Permalink
Merge pull request #314 from kevinrizza/index-generate-dameonless
Browse files Browse the repository at this point in the history
Bug 1829994: Index generate dameonless
  • Loading branch information
openshift-merge-robot authored May 1, 2020
2 parents 5c21e0f + d3c0563 commit e4773b9
Show file tree
Hide file tree
Showing 4 changed files with 79 additions and 30 deletions.
66 changes: 55 additions & 11 deletions cmd/opm/index/add.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,9 @@ func addIndexAddCmd(parent *cobra.Command) {
}
indexCmd.Flags().Bool("skip-tls", false, "skip TLS certificate verification for container image registries while pulling bundles")
indexCmd.Flags().StringP("binary-image", "i", "", "container image for on-image `opm` command")
indexCmd.Flags().StringP("container-tool", "c", "podman", "tool to interact with container images (save, build, etc.). One of: [docker, podman]")
indexCmd.Flags().StringP("container-tool", "c", "", "tool to interact with container images (save, build, etc.). One of: [docker, podman]")
indexCmd.Flags().StringP("build-tool", "u", "", "tool to build container images. One of: [docker, podman]. Defaults to podman. Overrides part of container-tool.")
indexCmd.Flags().StringP("pull-tool", "p", "", "tool to pull container images. One of: [none, docker, podman]. Defaults to none. Overrides part of container-tool.")
indexCmd.Flags().StringP("tag", "t", "", "custom tag for container image being built")
indexCmd.Flags().Bool("permissive", false, "allow registry load errors")
indexCmd.Flags().StringP("mode", "", "replaces", "graph update mode that defines how channel graphs are updated. One of: [replaces, semver, semver-skippatch]")
Expand Down Expand Up @@ -96,15 +98,6 @@ func runIndexAddCmdFunc(cmd *cobra.Command, args []string) error {
return err
}

containerTool, err := cmd.Flags().GetString("container-tool")
if err != nil {
return err
}

if containerTool == "none" {
return fmt.Errorf("none is not a valid container-tool for index add")
}

tag, err := cmd.Flags().GetString("tag")
if err != nil {
return err
Expand All @@ -130,11 +123,19 @@ func runIndexAddCmdFunc(cmd *cobra.Command, args []string) error {
return err
}

pullTool, buildTool, err := getContainerTools(cmd)
if err != nil {
return err
}

logger := logrus.WithFields(logrus.Fields{"bundles": bundles})

logger.Info("building the index")

indexAdder := indexer.NewIndexAdder(containertools.NewContainerTool(containerTool, containertools.PodmanTool), logger)
indexAdder := indexer.NewIndexAdder(
containertools.NewContainerTool(buildTool, containertools.PodmanTool),
containertools.NewContainerTool(pullTool, containertools.NoneTool),
logger)

request := indexer.AddToIndexRequest{
Generate: generate,
Expand All @@ -155,3 +156,46 @@ func runIndexAddCmdFunc(cmd *cobra.Command, args []string) error {

return nil
}

// getContainerTools returns the pull and build tools based on command line input
// to preserve backwards compatibility and alias the legacy `container-tool` parameter
func getContainerTools(cmd *cobra.Command) (string, string, error) {
buildTool, err := cmd.Flags().GetString("build-tool")
if err != nil {
return "", "", err
}

if buildTool == "none" {
return "", "", fmt.Errorf("none is not a valid container-tool for index add")
}

pullTool, err := cmd.Flags().GetString("pull-tool")
if err != nil {
return "", "", err
}

containerTool, err := cmd.Flags().GetString("container-tool")
if err != nil {
return "", "", err
}

// Backwards compatiblity mode
if containerTool != "" {
if pullTool == "" && buildTool == "" {
return containerTool, containerTool, nil
} else {
return "", "", fmt.Errorf("container-tool cannot be set alongside pull-tool or build-tool")
}
}

// Check for defaults, then return
if pullTool == "" {
pullTool = "none"
}

if buildTool == "" {
buildTool = "podman"
}

return pullTool, buildTool, nil
}
19 changes: 10 additions & 9 deletions pkg/lib/indexer/indexer.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@ type ImageIndexer struct {
RegistryAdder registry.RegistryAdder
RegistryDeleter registry.RegistryDeleter
RegistryPruner registry.RegistryPruner
ContainerTool containertools.ContainerTool
BuildTool containertools.ContainerTool
PullTool containertools.ContainerTool
Logger *logrus.Entry
}

Expand Down Expand Up @@ -71,7 +72,7 @@ func (i ImageIndexer) AddToIndex(request AddToIndexRequest) error {
// this is in its own function context so that the deferred cleanup runs before we do a docker build
// which prevents the full contents of the previous image from being in the build context
var databasePath string
if err := func () error {
if err := func() error {
tmpDir, err := ioutil.TempDir("./", tmpDirPrefix)
if err != nil {

Expand Down Expand Up @@ -99,7 +100,7 @@ func (i ImageIndexer) AddToIndex(request AddToIndexRequest) error {
Permissive: request.Permissive,
Mode: request.Mode,
SkipTLS: request.SkipTLS,
ContainerTool: i.ContainerTool,
ContainerTool: i.PullTool,
}

// Add the bundles to the registry
Expand Down Expand Up @@ -152,7 +153,7 @@ func (i ImageIndexer) DeleteFromIndex(request DeleteFromIndexRequest) error {
// this is in its own function context so that the deferred cleanup runs before we do a docker build
// which prevents the full contents of the previous image from being in the build context
var databasePath string
if err := func () error {
if err := func() error {
tmpDir, err := ioutil.TempDir("./", tmpDirPrefix)
if err != nil {

Expand Down Expand Up @@ -228,7 +229,7 @@ func (i ImageIndexer) PruneFromIndex(request PruneFromIndexRequest) error {
// this is in its own function context so that the deferred cleanup runs before we do a docker build
// which prevents the full contents of the previous image from being in the build context
var databasePath string
if err := func () error {
if err := func() error {
tmpDir, err := ioutil.TempDir("./", tmpDirPrefix)
if err != nil {

Expand Down Expand Up @@ -292,13 +293,13 @@ func (i ImageIndexer) getDatabaseFile(workingDir, fromIndex string) (string, err

var reg image.Registry
var rerr error
switch i.ContainerTool {
switch i.PullTool {
case containertools.NoneTool:
reg, rerr = containerdregistry.NewRegistry(containerdregistry.WithLog(i.Logger))
case containertools.PodmanTool:
fallthrough
case containertools.DockerTool:
reg, rerr = execregistry.NewRegistry(i.ContainerTool, i.Logger)
reg, rerr = execregistry.NewRegistry(i.PullTool, i.Logger)
}
if rerr != nil {
return "", rerr
Expand Down Expand Up @@ -339,7 +340,7 @@ func copyDatabaseTo(databaseFile, targetDir string) (string, error) {
if err := os.MkdirAll(targetDir, 0777); err != nil {
return "", err
}
} else {
} else if err != nil {
return "", err
}

Expand All @@ -353,7 +354,7 @@ func copyDatabaseTo(databaseFile, targetDir string) (string, error) {
dbFile := path.Join(targetDir, defaultDatabaseFile)

// define the path to copy to the database/index.db file
to, err := os.OpenFile(dbFile, os.O_RDWR|os.O_CREATE, 0666)
to, err := os.OpenFile(dbFile, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0666)
if err != nil {
return "", err
}
Expand Down
20 changes: 12 additions & 8 deletions pkg/lib/indexer/interfaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,15 @@ type IndexAdder interface {
}

// NewIndexAdder is a constructor that returns an IndexAdder
func NewIndexAdder(containerTool containertools.ContainerTool, logger *logrus.Entry) IndexAdder {
func NewIndexAdder(buildTool, pullTool containertools.ContainerTool, logger *logrus.Entry) IndexAdder {
return ImageIndexer{
DockerfileGenerator: containertools.NewDockerfileGenerator(logger),
CommandRunner: containertools.NewCommandRunner(containerTool, logger),
LabelReader: containertools.NewLabelReader(containerTool, logger),
CommandRunner: containertools.NewCommandRunner(buildTool, logger),
LabelReader: containertools.NewLabelReader(pullTool, logger),
RegistryAdder: registry.NewRegistryAdder(logger),
ImageReader: containertools.NewImageReader(containerTool, logger),
ContainerTool: containerTool,
ImageReader: containertools.NewImageReader(pullTool, logger),
BuildTool: buildTool,
PullTool: pullTool,
Logger: logger,
}
}
Expand All @@ -42,7 +43,8 @@ func NewIndexDeleter(containerTool containertools.ContainerTool, logger *logrus.
LabelReader: containertools.NewLabelReader(containerTool, logger),
RegistryDeleter: registry.NewRegistryDeleter(logger),
ImageReader: containertools.NewImageReader(containerTool, logger),
ContainerTool: containerTool,
BuildTool: containerTool,
PullTool: containerTool,
Logger: logger,
}
}
Expand All @@ -59,7 +61,8 @@ func NewIndexExporter(containerTool containertools.ContainerTool, logger *logrus
CommandRunner: containertools.NewCommandRunner(containerTool, logger),
LabelReader: containertools.NewLabelReader(containerTool, logger),
ImageReader: containertools.NewImageReader(containerTool, logger),
ContainerTool: containerTool,
BuildTool: containerTool,
PullTool: containerTool,
Logger: logger,
}
}
Expand All @@ -76,7 +79,8 @@ func NewIndexPruner(containerTool containertools.ContainerTool, logger *logrus.E
LabelReader: containertools.NewLabelReader(containerTool, logger),
RegistryPruner: registry.NewRegistryPruner(logger),
ImageReader: containertools.NewImageReader(containerTool, logger),
ContainerTool: containerTool,
BuildTool: containerTool,
PullTool: containerTool,
Logger: logger,
}
}
4 changes: 2 additions & 2 deletions test/e2e/opm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ func buildIndexWith(containerTool string) error {
bundleImage + ":" + bundleTag2,
}
logger := logrus.WithFields(logrus.Fields{"bundles": bundles})
indexAdder := indexer.NewIndexAdder(containertools.NewContainerTool(containerTool, containertools.NoneTool), logger)
indexAdder := indexer.NewIndexAdder(containertools.NewContainerTool(containerTool, containertools.NoneTool), containertools.NewContainerTool(containerTool, containertools.NoneTool), logger)

request := indexer.AddToIndexRequest{
Generate: false,
Expand All @@ -109,7 +109,7 @@ func buildFromIndexWith(containerTool string) error {
bundleImage + ":" + bundleTag3,
}
logger := logrus.WithFields(logrus.Fields{"bundles": bundles})
indexAdder := indexer.NewIndexAdder(containertools.NewContainerTool(containerTool, containertools.NoneTool), logger)
indexAdder := indexer.NewIndexAdder(containertools.NewContainerTool(containerTool, containertools.NoneTool), containertools.NewContainerTool(containerTool, containertools.NoneTool), logger)

request := indexer.AddToIndexRequest{
Generate: false,
Expand Down

0 comments on commit e4773b9

Please sign in to comment.