Remove CSVConfig and use CSV Generator for configurable inputs and output #2511
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -17,13 +17,12 @@ package generate | |||||
| import ( | ||||||
| "fmt" | ||||||
| "io/ioutil" | ||||||
| "os" | ||||||
| "path/filepath" | ||||||
|
|
||||||
| "github.com/operator-framework/operator-sdk/internal/generate/gen" | ||||||
| gencatalog "github.com/operator-framework/operator-sdk/internal/generate/olm-catalog" | ||||||
| "github.com/operator-framework/operator-sdk/internal/scaffold" | ||||||
| "github.com/operator-framework/operator-sdk/internal/util/fileutil" | ||||||
| "github.com/operator-framework/operator-sdk/internal/util/k8sutil" | ||||||
| "github.com/operator-framework/operator-sdk/internal/util/projutil" | ||||||
|
|
@@ -37,8 +36,11 @@ type csvCmd struct { | |||||
| csvVersion string | ||||||
| csvChannel string | ||||||
| fromVersion string | ||||||
| operatorName string | ||||||
| outputDir string | ||||||
| deployDir string | ||||||
| apisDir string | ||||||
| crdDir string | ||||||
| updateCRDs bool | ||||||
| defaultChannel bool | ||||||
| } | ||||||
|
|
@@ -55,33 +57,133 @@ A CSV semantic version is supplied via the --csv-version flag. If your operator | |||||
| has already generated a CSV manifest you want to use as a base, supply its | ||||||
| version to --from-version. Otherwise the SDK will scaffold a new CSV manifest. | ||||||
|
|
||||||
hasbro17 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
| CSV input flags: | ||||||
| --deploy-dir: | ||||||
| The CSV's install strategy and permissions will be generated from the operator manifests | ||||||
| (Deployment and Role/ClusterRole) present in this directory. | ||||||
|
|
||||||
| --apis-dir: | ||||||
| The CSV annotation comments will be parsed from the Go types under this path to | ||||||
| fill out metadata for owned APIs in spec.customresourcedefinitions.owned. | ||||||
|
|
||||||
| --crd-dir: | ||||||
| The CSV's spec.customresourcedefinitions.owned field is generated from the CRD manifests | ||||||
| in this path.These CRD manifests are also copied over to the bundle directory if --update-crds is set. | ||||||
| Additionally the CR manifests will be used to populate the CSV example CRs. | ||||||
| `, | ||||||
| Example: ` | ||||||
| ##### Generate CSV from default input paths ##### | ||||||
| $ tree pkg/apis/ deploy/ | ||||||
| pkg/apis/ | ||||||
| ├── ... | ||||||
| └── cache | ||||||
| ├── group.go | ||||||
| └── v1alpha1 | ||||||
| ├── ... | ||||||
| └── memcached_types.go | ||||||
| deploy/ | ||||||
| ├── crds | ||||||
| │ ├── cache.example.com_memcacheds_crd.yaml | ||||||
| │ └── cache.example.com_v1alpha1_memcached_cr.yaml | ||||||
| ├── operator.yaml | ||||||
| ├── role.yaml | ||||||
| ├── role_binding.yaml | ||||||
| └── service_account.yaml | ||||||
|
|
||||||
| $ operator-sdk generate csv --csv-version=0.0.1 --update-crds | ||||||
| INFO[0000] Generating CSV manifest version 0.0.1 | ||||||
| ... | ||||||
|
|
||||||
| $ tree deploy/ | ||||||
| deploy/ | ||||||
| ... | ||||||
| ├── olm-catalog | ||||||
| │ └── memcached-operator | ||||||
| │ ├── 0.0.1 | ||||||
| │ │ ├── cache.example.com_memcacheds_crd.yaml | ||||||
| │ │ └── memcached-operator.v0.0.1.clusterserviceversion.yaml | ||||||
| │ └── memcached-operator.package.yaml | ||||||
| ... | ||||||
|
|
||||||
|
|
||||||
|
|
||||||
| ##### Generate CSV from custom input paths ##### | ||||||
| $ operator-sdk generate csv --csv-version=0.0.1 --update-crds \ | ||||||
| --deploy-dir=config --apis-dir=api --output-dir=production | ||||||
| INFO[0000] Generating CSV manifest version 0.0.1 | ||||||
| ... | ||||||
|
|
||||||
| $ tree config/ api/ production/ | ||||||
| config/ | ||||||
| ├── crds | ||||||
| │ ├── cache.example.com_memcacheds_crd.yaml | ||||||
| │ └── cache.example.com_v1alpha1_memcached_cr.yaml | ||||||
| ├── operator.yaml | ||||||
| ├── role.yaml | ||||||
| ├── role_binding.yaml | ||||||
| └── service_account.yaml | ||||||
| api/ | ||||||
| ├── ... | ||||||
| └── cache | ||||||
| ├── group.go | ||||||
| └── v1alpha1 | ||||||
| ├── ... | ||||||
| └── memcached_types.go | ||||||
| production/ | ||||||
| └── olm-catalog | ||||||
| └── memcached-operator | ||||||
| ├── 0.0.1 | ||||||
| │ ├── cache.example.com_memcacheds_crd.yaml | ||||||
| │ └── memcached-operator.v0.0.1.clusterserviceversion.yaml | ||||||
| └── memcached-operator.package.yaml | ||||||
| `, | ||||||
|
|
||||||
| RunE: func(cmd *cobra.Command, args []string) error { | ||||||
| if len(args) != 0 { | ||||||
| return fmt.Errorf("command %s doesn't accept any arguments", cmd.CommandPath()) | ||||||
| } | ||||||
| if err := c.validate(); err != nil { | ||||||
| return fmt.Errorf("error validating command flags: %v", err) | ||||||
| } | ||||||
|
|
||||||
| if err := projutil.CheckProjectRoot(); err != nil { | ||||||
| log.Warn("Could not detect project root. Ensure that this command " + | ||||||
| "runs from the project root directory.") | ||||||
| } | ||||||
|
|
||||||
| // Default for crd dir if unset | ||||||
| if c.crdDir == "" { | ||||||
| c.crdDir = c.deployDir | ||||||
|
There was a problem hiding this comment. If you set this dir explicitly you don't have to modify the signature of
Suggested change
There was a problem hiding this comment. It would still be necessary to change the signature. The signature change is mostly because the Also There was a problem hiding this comment. In that case, we should:
Where is There was a problem hiding this comment. @estroz If we're doing a shallow read instead I'll leave that util untouched and just do a simple read in There was a problem hiding this comment. Remove this default since it is now set during flag registration. |
||||||
| } | ||||||
| if err := c.run(); err != nil { | ||||||
| log.Fatal(err) | ||||||
| } | ||||||
| return nil | ||||||
| }, | ||||||
| } | ||||||
|
|
||||||
| cmd.Flags().StringVar(&c.csvVersion, "csv-version", "", | ||||||
| "Semantic version of the CSV") | ||||||
| if err := cmd.MarkFlagRequired("csv-version"); err != nil { | ||||||
| log.Fatalf("Failed to mark `csv-version` flag for `generate csv` subcommand as required: %v", err) | ||||||
| } | ||||||
| cmd.Flags().StringVar(&c.fromVersion, "from-version", "", | ||||||
| "Semantic version of an existing CSV to use as a base") | ||||||
|
|
||||||
| // TODO: Allow multiple paths | ||||||
| // Deployment and RBAC manifests might be in different dirs e.g kubebuilder | ||||||
| cmd.Flags().StringVar(&c.deployDir, "deploy-dir", "deploy", | ||||||
| `Project relative path to root directory for operator manifests (Deployment and RBAC)`) | ||||||
| cmd.Flags().StringVar(&c.apisDir, "apis-dir", filepath.Join("pkg", "apis"), | ||||||
| `Project relative path to root directory for API type defintions`) | ||||||
| // TODO: Allow multiple paths | ||||||
| // CRD and CR manifests might be in different dirs e.g kubebuilder | ||||||
| cmd.Flags().StringVar(&c.crdDir, "crd-dir", filepath.Join("deploy", "crds"), | ||||||
| `Project relative path to root directory for CRD and CR manifests`) | ||||||
|
|
||||||
| cmd.Flags().StringVar(&c.outputDir, "output-dir", scaffold.DeployDir, | ||||||
| "Base directory to output generated CSV. The resulting CSV bundle directory "+ | ||||||
| "will be \"<output-dir>/olm-catalog/<operator-name>/<csv-version>\".") | ||||||
| cmd.Flags().BoolVar(&c.updateCRDs, "update-crds", false, | ||||||
| "Update CRD manifests in deploy/{operator-name}/{csv-version} the using latest API's") | ||||||
| cmd.Flags().StringVar(&c.operatorName, "operator-name", "", | ||||||
|
|
@@ -96,61 +198,45 @@ Configure CSV generation by writing a config file 'deploy/olm-catalog/csv-config | |||||
| } | ||||||
|
|
||||||
| func (c csvCmd) run() error { | ||||||
| log.Infof("Generating CSV manifest version %s", c.csvVersion) | ||||||
|
|
||||||
| if c.operatorName == "" { | ||||||
| c.operatorName = filepath.Base(projutil.MustGetwd()) | ||||||
| } | ||||||
| cfg := gen.Config{ | ||||||
| OperatorName: c.operatorName, | ||||||
| // TODO(hasbro17): Remove the Input key map when the Generator input keys | ||||||
| // are removed in favour of config fields in the csvGenerator | ||||||
hasbro17 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
| Inputs: map[string]string{ | ||||||
| gencatalog.DeployDirKey: c.deployDir, | ||||||
| gencatalog.APIsDirKey: c.apisDir, | ||||||
| gencatalog.CRDsDirKey: c.crdDir, | ||||||
| }, | ||||||
| OutputDir: c.outputDir, | ||||||
| } | ||||||
|
|
||||||
| csv := gencatalog.NewCSV(cfg, c.csvVersion, c.fromVersion) | ||||||
|
There was a problem hiding this comment. I'm now thinking of using a generator type-specific type CSVOptions struct {
gen.Config
OutputDir string
CSVVersion string
FromVersion string
DeployDir string
APIsDir string
}
func NewCSV(opts CSVOptions) Generator {
...
}An options struct is idiomatic and makes it easier to deal with defaults. Setup is a little more verbose because you're not only sharing one config, with the tradeoffs of generator-specific configuration captured in one place (no constructor args + inputs map) and the nice properties of struct fields vs those of map indexing.
Using keyed inputs still makes sense; we'd move the key definitions to There was a problem hiding this comment. @estroz Agreed. I'd probably go even further to remove the Config altogether if there isn't much else besides the Inputs and OutputDir. However to avoid increasing the scope of this PR (and refactoring the CRD generator as well) I'd suggest using the Generator config as it is right now so we can get And I'll add a TODO (for a later PR) in the Generator Config to refactor out Inputs and OutputDir into generator specific options. WDYT? |
||||||
| if err := csv.Generate(); err != nil { | ||||||
| return fmt.Errorf("error generating CSV: %v", err) | ||||||
| } | ||||||
| pkg := gencatalog.NewPackageManifest(cfg, c.csvVersion, c.csvChannel, c.defaultChannel) | ||||||
| if err := pkg.Generate(); err != nil { | ||||||
| return fmt.Errorf("error generating package manifest: %v", err) | ||||||
| } | ||||||
|
|
||||||
| // Write CRD's to the new or updated CSV package dir. | ||||||
| if c.updateCRDs { | ||||||
| crdManifestSet, err := findCRDFileSet(c.crdDir) | ||||||
| if err != nil { | ||||||
| return fmt.Errorf("failed to update CRD's: %v", err) | ||||||
camilamacedo86 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
| } | ||||||
| // TODO: This path should come from the CSV generator field csvOutputDir | ||||||
| bundleDir := filepath.Join(c.outputDir, gencatalog.OLMCatalogChildDir, c.operatorName, c.csvVersion) | ||||||
| for path, b := range crdManifestSet { | ||||||
| path = filepath.Join(bundleDir, path) | ||||||
| if err = ioutil.WriteFile(path, b, fileutil.DefaultFileMode); err != nil { | ||||||
| return fmt.Errorf("failed to update CRD's: %v", err) | ||||||
hasbro17 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
| } | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
|
|
@@ -191,25 +277,45 @@ func validateVersion(version string) error { | |||||
| return nil | ||||||
| } | ||||||
|
|
||||||
| // findCRDFileSet searches files in the given directory path for CRD manifests, | ||||||
| // returning a map of paths to file contents. | ||||||
| func findCRDFileSet(crdDir string) (map[string][]byte, error) { | ||||||
| crdFileSet := map[string][]byte{} | ||||||
| info, err := os.Stat(crdDir) | ||||||
| if err != nil { | ||||||
| return nil, err | ||||||
| } | ||||||
| if !info.IsDir() { | ||||||
| return nil, fmt.Errorf("crd's must be read from a directory. %s is a file", crdDir) | ||||||
| } | ||||||
| files, err := ioutil.ReadDir(crdDir) | ||||||
| if err != nil { | ||||||
| return nil, err | ||||||
| } | ||||||
|
|
||||||
| wd := projutil.MustGetwd() | ||||||
| for _, f := range files { | ||||||
| if f.IsDir() { | ||||||
| continue | ||||||
| } | ||||||
|
|
||||||
| crdPath := filepath.Join(wd, crdDir, f.Name()) | ||||||
| b, err := ioutil.ReadFile(crdPath) | ||||||
hasbro17 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
| if err != nil { | ||||||
| return nil, fmt.Errorf("error reading manifest %s: %v", crdPath, err) | ||||||
| } | ||||||
| // Skip files in crdsDir that aren't k8s manifests since we do not know | ||||||
| // what other files are in crdsDir. | ||||||
| typeMeta, err := k8sutil.GetTypeMetaFromBytes(b) | ||||||
| if err != nil { | ||||||
| log.Debugf("Skipping non-manifest file %s: %v", crdPath, err) | ||||||
| continue | ||||||
| } | ||||||
| if typeMeta.Kind != "CustomResourceDefinition" { | ||||||
| log.Debugf("Skipping non CRD manifest %s", crdPath) | ||||||
| continue | ||||||
| } | ||||||
| crdFileSet[filepath.Base(crdPath)] = b | ||||||
| } | ||||||
| return crdFileSet, nil | ||||||
| } | ||||||
Uh oh!
There was an error while loading. Please reload this page.