cmd/operator-sdk/olmcatalog/gen-csv.go: --write-config to write CSV config to disk #1346

estroz · 2019-04-25T00:00:00Z

Description of the change:
internal/pkg/scaffold/olm-catalog: Use a CSVConfigFile in a CSV scaffold directly instead of reading from a file. CSVConfig is now a scaffold that contains a CSVConfigFile with the actual data to write.

cmd/operator-sdk/olmcatalog/gen-csv.go: Add --write-csv-config flag to both olm-catalog and olm-catalog gen-csv so users can write a CSV config to a default path or the one passed to --write-csv-config.

Motivation for the change: #1259 does not directly pertain to the changes here but brought to my attention that CSV config files are not well understood. --write-config should help because it creates a default config with all fields populated.

estroz · 2019-04-25T18:04:06Z

/retest

estroz · 2019-04-25T23:11:38Z

After thinking about this further, its probably better to have a olm-catalog write-csv-config <path> command or olm-catalog --write-csv-config=<path> so a user doesn't have to (re)write their CSV just to get a config.

scaffold directly instead of reading from a file. CSVConfig is now a scaffold-able file. cmd/operator-sdk/olmcatalog: add the --write-csv-config flag so users can write a default CSV config to a default or custom path

…d add notes on flags

…ig in 'olm-catalog gen-csv'

…atalog gen-csv'

estroz · 2019-05-03T18:13:03Z

/hold

Going to wait on this until we figure out configuration file issues.

openshift-ci-robot · 2019-05-12T02:47:44Z

@estroz: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-ci-robot · 2019-08-09T18:25:12Z

@estroz: The following tests failed, say /retest to rerun them all:

Test name	Commit	Details	Rerun command
ci/prow/images	`b677bd5`	link	`/test images`
ci/prow/e2e-aws-ansible	`b677bd5`	link	`/test e2e-aws-ansible`
ci/prow/e2e-aws-helm	`b677bd5`	link	`/test e2e-aws-helm`
ci/prow/e2e-aws-go	`b677bd5`	link	`/test e2e-aws-go`
ci/prow/e2e-aws-subcommand	`b677bd5`	link	`/test e2e-aws-subcommand`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

openshift-bot · 2019-11-07T23:23:42Z

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

camilamacedo86 · 2019-11-08T10:22:17Z

/remove-lifecycle

camilamacedo86 · 2019-11-19T10:20:40Z

cmd/operator-sdk/olmcatalog/gen-csv.go

@@ -56,18 +57,18 @@ Configure CSV generation by writing a config file 'deploy/olm-catalog/csv-config
 	genCSVCmd.Flags().StringVar(&csvVersion, "csv-version", "", "Semantic version of the CSV")
 	genCSVCmd.MarkFlagRequired("csv-version")
 	genCSVCmd.Flags().StringVar(&fromVersion, "from-version", "", "Semantic version of an existing CSV to use as a base")
-	genCSVCmd.Flags().StringVar(&csvConfigPath, "csv-config", "", "Path to CSV config file. Defaults to deploy/olm-catalog/csv-config.yaml")
+	genCSVCmd.Flags().StringVar(&csvConfigPath, "csv-config", "", "Path to CSV config file. If unset, default file paths are used")


I think is good to keep the default path in the info.

camilamacedo86 · 2019-11-19T10:22:00Z

cmd/operator-sdk/olmcatalog/cmd.go

 	"github.com/spf13/cobra"
 )

+var (
+	writeCSVConfigOpt = "write-csv-config"
+


camilamacedo86 · 2019-11-19T10:24:50Z

cmd/operator-sdk/olmcatalog/cmd.go

+	i, err := (&catalog.CSVConfig{}).GetInput()
+	if err != nil {
+		log.Fatalf("Error retrieving CSV config path: %v", err)
+	}


Could not the path be an attribute of the struct and the check be made in the Validate func of it?

camilamacedo86 · 2019-11-19T10:26:29Z

cmd/operator-sdk/olmcatalog/cmd.go

+			return err
+		}
+	}
+	return nil


Is it creating a new input.Config? Could not it be a NewConfig in its struct, or at least has a more meaningful name?

camilamacedo86 · 2019-11-19T10:27:29Z

cmd/operator-sdk/olmcatalog/cmd.go

+	}
+	return nil
+}
+func writeConfig(s *scaffold.Scaffold, cfg *input.Config) error {


Suggested change

func writeConfig(s *scaffold.Scaffold, cfg *input.Config) error {

func writeCSVConfig(s *scaffold.Scaffold, cfg *input.Config) error {

camilamacedo86 · 2019-11-19T10:49:59Z

cmd/operator-sdk/olmcatalog/gen-csv.go

+	} else {
+		log.Info("Using default CSV config.")
+		csvCfgFile = &catalog.CSVConfigFile{}
+	}



could not it be improved?
By default, the default CSVFile be set then if do not find it or the customized one return an error?
Could not the logic be when the new struct is created?

estroz added kind/feature Categorizes issue or PR as related to a new feature. olm-integration Issue relates to the OLM integration labels Apr 25, 2019

estroz requested review from joelanford, AlexNPavel, lilic, hasbro17 and theishshah April 25, 2019 00:00

openshift-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Apr 25, 2019

estroz changed the title ~~Csv config optional crdsdir~~ cmd/operator-sdk/olmcatalog/gen-csv.go: --write-config to write CSV config to disk Apr 25, 2019

estroz force-pushed the csv-config-optional-crdsdir branch from b531638 to bf49c18 Compare April 25, 2019 01:19

estroz added 4 commits April 26, 2019 12:49

internal/pkg/scaffold/olm-catalog: Use a CSVConfigFile in a CSV

21e44e0

scaffold directly instead of reading from a file. CSVConfig is now a scaffold-able file. cmd/operator-sdk/olmcatalog: add the --write-csv-config flag so users can write a default CSV config to a default or custom path

doc/user/olm-catalog/generating-a-csv.md: update config file paths an…

61d780c

…d add notes on flags

doc/sdk-cli-reference.md: add --write-config to and modify --csv-conf…

e4a9779

…ig in 'olm-catalog gen-csv'

CHANGELOG.md: add --write-config to and modify --csv-config in 'olm-c…

b677bd5

…atalog gen-csv'

estroz force-pushed the csv-config-optional-crdsdir branch from 1a8a666 to b677bd5 Compare April 26, 2019 19:52

openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 3, 2019

openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 12, 2019

openshift-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Nov 7, 2019

camilamacedo86 reviewed Nov 19, 2019

View reviewed changes

cmd/operator-sdk/olmcatalog/cmd.go

"github.com/spf13/cobra"

)

var (

writeCSVConfigOpt = "write-csv-config"

Copy link

Contributor

camilamacedo86 Nov 19, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

space

camilamacedo86 reviewed Nov 19, 2019

View reviewed changes

estroz mentioned this pull request Nov 21, 2019

cmd/.../gen-csv: use --include instead of CSV config #2249

Closed

estroz closed this Nov 21, 2019

estroz deleted the csv-config-optional-crdsdir branch April 1, 2020 22:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

cmd/operator-sdk/olmcatalog/gen-csv.go: --write-config to write CSV config to disk #1346

cmd/operator-sdk/olmcatalog/gen-csv.go: --write-config to write CSV config to disk #1346

Uh oh!

estroz commented Apr 25, 2019 •

edited

Loading

Uh oh!

estroz commented Apr 25, 2019

Uh oh!

estroz commented Apr 25, 2019 •

edited

Loading

Uh oh!

estroz commented May 3, 2019

Uh oh!

openshift-ci-robot commented May 12, 2019

Uh oh!

openshift-ci-robot commented Aug 9, 2019

Uh oh!

openshift-bot commented Nov 7, 2019

Uh oh!

camilamacedo86 commented Nov 8, 2019

Uh oh!

camilamacedo86 Nov 19, 2019

Uh oh!

camilamacedo86 Nov 19, 2019

Uh oh!

camilamacedo86 Nov 19, 2019

Uh oh!

camilamacedo86 Nov 19, 2019 •

edited

Loading

Uh oh!

camilamacedo86 Nov 19, 2019

Uh oh!

camilamacedo86 Nov 19, 2019

Uh oh!

Uh oh!

	func writeConfig(s scaffold.Scaffold, cfg input.Config) error {
	func writeCSVConfig(s scaffold.Scaffold, cfg input.Config) error {

cmd/operator-sdk/olmcatalog/gen-csv.go: --write-config to write CSV config to disk #1346

cmd/operator-sdk/olmcatalog/gen-csv.go: --write-config to write CSV config to disk #1346

Uh oh!

Conversation

estroz commented Apr 25, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

estroz commented Apr 25, 2019

Uh oh!

estroz commented Apr 25, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

estroz commented May 3, 2019

Uh oh!

openshift-ci-robot commented May 12, 2019

Uh oh!

openshift-ci-robot commented Aug 9, 2019

Uh oh!

openshift-bot commented Nov 7, 2019

Uh oh!

camilamacedo86 commented Nov 8, 2019

Uh oh!

camilamacedo86 Nov 19, 2019

Choose a reason for hiding this comment

Uh oh!

camilamacedo86 Nov 19, 2019

Choose a reason for hiding this comment

Uh oh!

camilamacedo86 Nov 19, 2019

Choose a reason for hiding this comment

Uh oh!

camilamacedo86 Nov 19, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

camilamacedo86 Nov 19, 2019

Choose a reason for hiding this comment

Uh oh!

camilamacedo86 Nov 19, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

estroz commented Apr 25, 2019 •

edited

Loading

estroz commented Apr 25, 2019 •

edited

Loading

camilamacedo86 Nov 19, 2019 •

edited

Loading