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

Create a template for configuring multiple applications to be run with dapr run command #1141

Closed
mukundansundar opened this issue Dec 6, 2022 · 10 comments · Fixed by #1152
Closed
Assignees
Labels
area/cli kind/feature New feature or request P1 size/S 1 week of work
Milestone

Comments

@mukundansundar
Copy link
Collaborator

mukundansundar commented Dec 6, 2022

Describe the proposal

As part of implementing dapr/proposals#6:

  • create a template based on seen in the proposal.
  • Implement YAML template processing with UTs for the run configuration file.

Example

version: 1

common: (optional)

  resources_dir: ./app/components # any dapr resources to be shared across apps

  env:  # any environment variable shared among apps

    - DEBUG: true

apps:

  - app_id: webapp ## required

    app_dir: ./webapp/ ## required

    resources_dir: ./webapp/components # (optional) can be default by convention too, ignore if dir is not found.

    config_file: ./webapp/config.yaml # (optional) can be default by convention too, ignore if file is not found.

    app_protocol: HTTP

    app_port: 8080

    app_health_check_path: "/healthz" # All _ converted to - for all properties defined under daprd section

    command: ["python3" "app.py"]

  - app_id: backend

    app_dir: ./backend/

    app_protocol: GRPC

    app_port: 3000

    unix_domain_socket: "/tmp/test-socket"

    env:

      - DEBUG: false

    command: ["./backend"]

A single run configuration file contains a single YAML definition.

Minimum validations to be done :

  • YAML struct is correct
  • required fileds are there
  • paths are reachable and already there

related to #1123

Release Note

RELEASE NOTE:

@mukundansundar mukundansundar added kind/feature New feature or request area/cli P1 size/S 1 week of work kind/proposal A new proposal to be considered labels Dec 6, 2022
@mukundansundar mukundansundar added this to the v1.10 milestone Dec 6, 2022
@mukundansundar mukundansundar removed the kind/proposal A new proposal to be considered label Dec 6, 2022
@pravinpushkar
Copy link
Contributor

/assign

@pravinpushkar
Copy link
Contributor

pravinpushkar commented Dec 14, 2022

This issue contains details for parsing the provided yaml file when dapr run -f <filename.yaml> is executed. The main goals would be -

  1. Validate the yaml file and parse the provided yaml file to a struct.
  2. Validate the required fields are present.
  3. All paths mentioned in the file should be reachable.

We can use below structs to capture this -

type AppsRunConfig struct {
	Common  Common `yaml:"common"`
	Apps    []Apps `yaml:"apps"`
	Version int    `yaml:"version"`
}

type Apps struct {
	AppId       string            `yaml:"app_id"`
	AppDir      string            `yaml:"app_dir"`
	Command     []string          `yaml:"command"`
	AppEnvs     map[string]string `yaml:"app_envs"`
	RunCMDFlags map[string]string `yaml:"run_cmd_flags"`
}

type Common struct {
	SharedCMDFlags map[string]string `yaml:"shared_properties"`
	SharedEnvs       map[string]string `yaml:"shared_envs"`
}

@yaron2 @mukundansundar @shubham1172

@shubham1172
Copy link
Member

shubham1172 commented Dec 14, 2022

@pravinpushkar how will we validate RunCMDFlags and SharedProperties? Currently we store all the flags in the RunConfig struct. How about this structure?

type AppsRunConfig struct {
	Common  Common `yaml:"common"`
	Apps    []Apps `yaml:"apps"`
	Version int    `yaml:"version"`
}

// Apps can contain App specific settings + embed Common
type Apps struct {
        Common
	AppId        string            `yaml:"app_id"`
	AppDir      string            `yaml:"app_dir"`
	Command     []string          `yaml:"command"`
}

// Common can contain everything that can be shared between multiple apps
type Common struct {
	Env       map[string]string `yaml:"shared_envs"`
        ResourcesPath string `yaml:"resources-path"`
        // ... others
}

And finally for each app, one needs to append values in Apps[i].Common with Common.

@pravinpushkar
Copy link
Contributor

one correction here - SharedProperties is actually SharedCMDFlags.

how will we validate RunCMDFlags and SharedProperties

I was thinking, when we have translated the yaml into below struct -

type AppsRunConfig struct {
	Common  Common `yaml:"common"`
	Apps    []Apps `yaml:"apps"`
	Version int    `yaml:"version"`
}

For each app, we can call the final RunConfig struct which will contain merged values from SharedCMDFlags and RunCMDFlags and similarly merged values from SharedEnvs and AppEnvs and let the current validations take care of everything.

I got your idea also, which basically differs in the use of above flags and env and instead use common.
This also seems fine.
Only issue I can see is we are maintaining the list of supported flags here also along with Runconfig struct. OR we somehow find a way to refactor the current RunConfig to be utilized here also. Thoughts?

@shubham1172
Copy link
Member

Only issue I can see is we are maintaining the list of supported flags here also along with Runconfig struct. OR we somehow find a way to refactor the current RunConfig to be utilized here also. Thoughts?

Yes, if we do this, RunConfig should be updated accordingly. We should strive to generalize the structs used in running single app with vanilla dapr run and running multiple apps.

@pravinpushkar
Copy link
Contributor

@pravinpushkar how will we validate RunCMDFlags and SharedProperties? Currently we store all the flags in the RunConfig struct. How about this structure?

type AppsRunConfig struct {
	Common  Common `yaml:"common"`
	Apps    []Apps `yaml:"apps"`
	Version int    `yaml:"version"`
}

// Apps can contain App specific settings + embed Common
type Apps struct {
        Common
	AppId        string            `yaml:"app_id"`
	AppDir      string            `yaml:"app_dir"`
	Command     []string          `yaml:"command"`
}

// Common can contain everything that can be shared between multiple apps
type Common struct {
	Env       map[string]string `yaml:"shared_envs"`
        ResourcesPath string `yaml:"resources-path"`
        // ... others
}

And finally for each app, one needs to append values in Apps[i].Common with Common.

While implementing I found this easier and logical, so proceeding with this only for now. I will post if there is any change.

@mukundansundar
Copy link
Collaborator Author

@msfussell @yaron2 @greenie-msft
In this change, we are asking users to specify a resources_dir field for the directory of the resources. We also ask the user to provide an app_dir for the application directory.

From existing arguments in CLI master branch and daprd, we have the field called as --resources-path.
Which name should be used for the resources directory?
Should it directly be resources_dir or if we say it is path to the resources dir so it should be resources_path, then in that scenario should the application directory also be renamed as app_dir_path as in path to the application directory?

@msfussell
Copy link
Member

A much as possible we should have identical names between the CLI and the template. So these names should be used https://docs.dapr.io/reference/cli/dapr-run/. The name then should be resources_path.
Also is there any reason to use underscores "_" rather than a dash "-" in the template file? In other words could we have resources-path to make this even more CLI aligned?

@mukundansundar
Copy link
Collaborator Author

sh "-" in the template file? In other words could we have resources-path to make this even more CLI aligned?

Ideally separators in YAML are always _ and not -.

So then will we have app_dir_path for the path to the application dir?

@msfussell
Copy link
Member

sh "-" in the template file? In other words could we have resources-path to make this even more CLI aligned?

Ideally separators in YAML are always _ and not -.

So then will we have app_dir_path for the path to the application dir?

Yes to app_dir_path

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/cli kind/feature New feature or request P1 size/S 1 week of work
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants