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.

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

Conversation

ruflin
Copy link
Member

@ruflin ruflin commented Apr 24, 2023

The idea behind this pull request is to be able to run a serverless stack of Elasticsearch with the following command:

elastic-package stack up --version=8.8.0-SNAPSHOT -v --serverless

I'm opening this PR as draft as I have a few questions on how to implement it best.

The idea behind this pull request is to be able to run a `serverless` stack of Elasticsearch with the following command:

```
elastic-package stack up --version=8.8.0-SNAPSHOT -v --serverless
```

I'm opening this PR as draft as I have a few questions on how to implement it best.
@@ -73,6 +73,11 @@ func setupStackCommand() *cobraext.Command {
return cobraext.FlagParsingError(err, cobraext.StackVersionFlagName)
}

/*serverless, err := cmd.Flags().GetString(cobraext.ServerlessFlagName)
Copy link
Member 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
Member 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
Member 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.

@@ -47,6 +47,8 @@ const (

elasticsearchUsername = "elastic"
elasticsearchPassword = "changeme"

serverless = "false"
Copy link
Member 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.

@ruflin ruflin self-assigned this Apr 24, 2023
@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

cc @ruflin

@ruflin
Copy link
Member Author

ruflin commented Apr 27, 2023

For reference as this is related: elastic/kibana#155009 I assume we will not only start Elasticsearch in serverless but also Kibana.

@sharbuz
Copy link
Contributor

sharbuz commented Jul 10, 2023

Hi everyone,
Please update the branch because we merged an important change to the main branch: #1347

@ruflin
Copy link
Member Author

ruflin commented Jul 18, 2023

Currently closing this as I didn't find the time to work on this. @jsoriano I still think this would be useful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants