Skip to content
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.

Sign up for GitHub

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

Add serverless flag to stack up #1231

Closed
wants to merge 1 commit into from
Closed

Add serverless flag to stack up #1231

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions cmd/stack.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,11 @@ func setupStackCommand() *cobraext.Command {
return cobraext.FlagParsingError(err, cobraext.StackVersionFlagName)
}

/*serverless, err := cmd.Flags().GetString(cobraext.ServerlessFlagName)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mrodm @jsoriano Any recommendations on the path to follow here. I'm looking at geoip_dir which is part of a profile. At the same time I don't know if making it part of profiles is overkill?

As you see further down, it is a variable in the template loaded from the resources. How do I set the resource variable on elastic-stack up in the best way? https://github.com/elastic/elastic-package/pull/1231/files#diff-9fd733fc9d8f10a5c2dd55a062523d3dcb0655bc1db097648b3adabbd3460173R123

Copy link
Member

Choose a reason for hiding this comment

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

Hey, thanks for adding this. I would add it as a profile setting, in the same way as geoip_dir. Maybe in the future we can add a flag to pass this kind of settings through the command line, so we can run commands like elastic-package stack up -Dstack.serverless=true.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I assume this would mean it can only be used by someone that adds a custom profile to elastic-package? The part I'm worried with this is that the feature will be tricky to use. I want to make it as easy as possible for everyone to test serverless by just adding --serverless like other params.

Copy link
Member

Choose a reason for hiding this comment

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

It is kind of a parameter for the "compose" stack provider, integrating it with the framework for settings we are preparing will also make it persistent, so users will only need to run something like:

elastic-package profiles create local-serverless
elastic-package profiles use local-serverless
elastic-package stack up -Dstack.serverless=true

We are preparing a cloud stack provider (#1230), and we will probably prepare a "serverless" one once the offering is ready. It may be confusing to have a --serverless flag and a --compose serverless flag.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If my understand is correct, the path forward is that we have multiple profiles available by default meaning users of elastic-package are likely already familiar by then with profiles. With this in mind, what you suggest makes a lot of sense. So if I want to switch between setups, I just switch the profiles.

Copy link
Member

Choose a reason for hiding this comment

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

We don't ship predefined profiles (apart of the default one), but this would be definitely an option once we have more options.

if err != nil {
return cobraext.FlagParsingError(err, cobraext.ServerlessFlagName)
}*/

profile, err := getProfileFlag(cmd)
if err != nil {
return err
Expand Down
3 changes: 3 additions & 0 deletions internal/cobraext/flags.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,9 @@ const (
ZipPackageFilePathFlagShorthand = "z"
ZipPackageFilePathFlagDescription = "path to the zip package file (*.zip)"

ServerlessFlagName = "serverless"
ServerlessFlagDescription = "serverless flag (true or false)"

// To be removed promote commands flags
DirectionFlagName = "direction"
DirectionFlagDescription = "promotion direction"
Expand Down
3 changes: 2 additions & 1 deletion internal/stack/_static/docker-compose-stack.yml.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
{{ $username := fact "username" }}
{{ $password := fact "password" }}
{{ $serverless := fact "serverless" }}
version: '2.3'
services:
elasticsearch:
Expand All @@ -9,7 +10,7 @@ services:
start_period: 300s
interval: 5s
environment:
- "ES_JAVA_OPTS=-Xms1g -Xmx1g"
- "ES_JAVA_OPTS=-Xms1g -Xmx1g -Des.serverless={{ $serverless }}"
- "ELASTIC_PASSWORD={{ $password }}"
volumes:
- "./elasticsearch.yml:/usr/share/elasticsearch/config/elasticsearch.yml"
Expand Down
4 changes: 4 additions & 0 deletions internal/stack/resources.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ const (

elasticsearchUsername = "elastic"
elasticsearchPassword = "changeme"

serverless = "false"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Setting this to true manually leads to the expected result.

)

var (
Expand Down Expand Up @@ -118,6 +120,8 @@ func applyResources(profile *profile.Profile, stackVersion string) error {
"username": elasticsearchUsername,
"password": elasticsearchPassword,

"serverless": serverless,

"geoip_dir": profile.Config("stack.geoip_dir", "./ingest-geoip"),
})

Expand Down