-
Notifications
You must be signed in to change notification settings - Fork 787
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add support for helmfile in the apps model #6660
Changes from all commits
ce6ab1c
3e540a9
c441ae3
ab161fa
d9f504f
f05a967
2cd5171
5ae4e0b
8820ffa
a81936e
4968833
ca0939d
3209fe7
c7d0454
bce4142
e213c49
84c5288
61a2300
b0ed159
272c3d5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,7 +4,6 @@ import ( | |
"fmt" | ||
|
||
"github.com/jenkins-x/jx/pkg/cmd/helper" | ||
|
||
"github.com/jenkins-x/jx/pkg/cmd/opts" | ||
"github.com/jenkins-x/jx/pkg/cmd/templates" | ||
|
||
|
@@ -61,6 +60,9 @@ var ( | |
# Add an app | ||
jx add app jx-app-jacoco | ||
|
||
# Add an app using a chart repository prefix from the version stream 'charts/repositories.yml' file | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here things get confusing for me. So far this command is documented as being responsible for installing "apps" which are similar to "addons". So without any further context, I read this as "install an addon from a chart repository prefix from the version stream 'charts/repositories.yml' file". And not sure what the second part really means. Do you have a plan how we get out of this naming mess? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the plan is to always use app and remove addon: https://github.com/jenkins-x/enhancements/tree/master/proposals/1 |
||
jx add app flagger/flagger | ||
|
||
# Add an app from a local path | ||
jx add app .`) | ||
) | ||
|
@@ -116,14 +118,12 @@ func (o *AddAppOptions) addFlags(cmd *cobra.Command, defaultNamespace string) { | |
|
||
// Run implements this command | ||
func (o *AddAppOptions) Run() error { | ||
o.GitOps, o.DevEnv = o.GetDevEnv() | ||
if o.Repo == "" { | ||
o.Repo = o.DevEnv.Spec.TeamSettings.AppsRepository | ||
} | ||
if o.Repo == "" { | ||
o.Repo = kube.DefaultChartMuseumURL | ||
ec, err := o.EnvironmentContext(".", false) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
o.GitOps = ec.GitOps | ||
o.DevEnv = ec.DevEnv | ||
jxClient, ns, err := o.JXClientAndDevNamespace() | ||
if err != nil { | ||
return errors.Wrapf(err, "getting jx client") | ||
|
@@ -135,6 +135,7 @@ func (o *AddAppOptions) Run() error { | |
if o.Namespace == "" { | ||
o.Namespace = ns | ||
} | ||
|
||
installOpts := apps.InstallOptions{ | ||
IOFileHandles: o.GetIOFileHandles(), | ||
DevEnv: o.DevEnv, | ||
|
@@ -150,6 +151,7 @@ func (o *AddAppOptions) Run() error { | |
JxClient: jxClient, | ||
InstallTimeout: opts.DefaultInstallTimeout, | ||
EnvironmentCloneDir: o.CloneDir, | ||
VersionResolver: ec.VersionResolver, | ||
} | ||
|
||
if o.GitOps { | ||
|
@@ -214,16 +216,17 @@ func (o *AddAppOptions) Run() error { | |
return o.Cmd.Help() | ||
} | ||
|
||
if o.Repo == "" { | ||
return fmt.Errorf("must specify a repository") | ||
} | ||
app := args[0] | ||
|
||
var version string | ||
if o.Version != "" { | ||
version = o.Version | ||
details, err := ec.ChartDetails(app, o.Repo) | ||
if err != nil { | ||
return err | ||
} | ||
app := args[0] | ||
return installOpts.AddApp(app, version, o.Repo, o.Username, o.Password, o.ReleaseName, o.ValuesFiles, o.SetValues, | ||
if details.Repository == "" { | ||
return fmt.Errorf("must specify a repository or use a repository prefix in the chart name %s", details.Name) | ||
} | ||
o.Repo = details.Repository | ||
return installOpts.AddApp(details, o.Version, o.Username, o.Password, o.ReleaseName, o.ValuesFiles, o.SetValues, | ||
o.Alias, o.HelmUpdate) | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So
GetApplication
so far where the user's "applications" right? Now you want to call this "Apps" and unify this with the current Apps plugins and all become apps? We really are in a bit of a naming mess here.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see https://github.com/jenkins-x/enhancements/tree/master/proposals/1 but I'm also hoping to submit a further enhancement to address the mess of
jx get app
vjx get applications
but that would be another enhancement + PR etc...