Skip to content
This repository was archived by the owner on May 18, 2026. It is now read-only.

Allow pass maven artifact through flags #256 #259

Merged
atmandhol merged 2 commits into
vmware-tanzu:mainfrom
shashwathi:iss-170
Aug 17, 2022
Merged

Allow pass maven artifact through flags #256 #259
atmandhol merged 2 commits into
vmware-tanzu:mainfrom
shashwathi:iss-170

Conversation

@shashwathi
Copy link
Copy Markdown
Contributor

What this PR does / why we need it

Allows passing maven info through flags.

  • If flags are set, these will take priority over paramsyaml
  • maven source can be updated partially (no need for all the flags) only through flags, if just one of the items is to be updated, don't use paramsyaml since it's going to delete the ones that are not updated

Which issue(s) this PR fixes

Fixes #170

@shashwathi shashwathi marked this pull request as ready for review August 11, 2022 04:44
@atmandhol atmandhol requested a review from anibmurthy August 11, 2022 13:22
@shashwathi shashwathi force-pushed the iss-170 branch 2 times, most recently from c49ec51 to e4ad3b3 Compare August 11, 2022 18:58
Comment thread pkg/commands/workload.go
@shashwathi
Copy link
Copy Markdown
Contributor Author

/rebase

Wendy Arango and others added 2 commits August 16, 2022 22:33
Signed-off-by: Wendy Arango <warango@vmware.com>
- add unit tests
- update validation for source check to include maven param
- add e2e test for maven

Signed-off-by: Shash Reddy <snagarajared@vmware.com>
Copy link
Copy Markdown
Contributor

@anibmurthy anibmurthy left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link
Copy Markdown
Contributor

@shaheerkootteeri shaheerkootteeri left a comment

Choose a reason for hiding this comment

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

A clarification in the code

Comment thread pkg/apis/cartographer/v1alpha1/workload_helpers.go
@atmandhol atmandhol merged commit 74cde3b into vmware-tanzu:main Aug 17, 2022
@shashwathi shashwathi deleted the iss-170 branch August 17, 2022 18:19
@heyjcollins
Copy link
Copy Markdown
Contributor

AC

  1. confirmed presence and accuracy of --maven* flags and descriptions in tanzu apps workload create/update/apply help output
  2. confirmed maven-based workload creation via --params-yaml hasn't regressed
  3. confirmed presence of >= 1 --maven* flag overrides --params-yaml completely
  4. confirmed validation is taking place for the 3 required maven flags (--maven-artifact, --maven-group, --maven-version)
  5. confirmed it's possible to execute a partial update of an existing maven-based workload (can pass in less than all the required maven flags, pass validation, and just update the workload with the changed maven value(s)

LGTM overall!

Note: if all the required maven flags are provided and --maven-type isn't included, the plugin still adds spec.params.maven.type: null to the workload object. This isn't expected, however, the current backend will accept this without error so the current implementation is acceptable and we'll clean that up as a follow up change (will log a separate issue).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Maven source is supported via command flags (for workload create & apply)

7 participants