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

VirtualDomain: New resource attribute "start_scripts" #857

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

berlevdv
Copy link
Contributor

Create new resource attribute "start_scripts" for control domain start via scripts.

@berlevdv berlevdv changed the title New resource attribute "start_scripts" VirtualDomain: New resource attribute "start_scripts" Oct 10, 2016
@krig
Copy link
Contributor

krig commented Oct 20, 2016

Why not create a separate resource for your start script (using the anything resource agent) and group that resource with your VirtualDomain resource? That way, your script will run first and only if it starts successfully will the VirtualDomain resource start.

No need to add the "start_scripts" parameter to VirtualDomain.

@berlevdv
Copy link
Contributor Author

I thought that once used the "monitor scripts", which functionality can be used by "anything" resource agent, it would be a good idea here is to implement the "start scripts". In addition it will prevent "excess" resources, their setting and specify same configuration file path at different resources.

@krig
Copy link
Contributor

krig commented Nov 2, 2016

The API for start_scripts is not the same as for monitor_scripts: It looks like a start script takes a domain name as parameter, while the monitor scripts seem to not take any parameters. Is this correct?

Either way, the difference should be documented in the longdesc for start_scripts, at least.

@berlevdv
Copy link
Contributor Author

Hi, Krig! Yes, start script section of VirtualDomain_start() function pass $DOMAIN_NAME variable as parameter to script for domain identification and further usage. With this option there is no need to manually specify the domain name (which may change in the future) when defining the VirtualDomain resource, such as in monitor_scripts.

I will add describe of this option in the longdesc for start_scripts. Thanks!

@krig
Copy link
Contributor

krig commented Nov 15, 2016

Alright, looks good to me now.

@krig
Copy link
Contributor

krig commented Nov 15, 2016

Could you collapse the commits into a single commit with a commit title like

Medium: VirtualDomain: Add start_scripts parameter

?

To do this, check out your branch (master in this case) and then run

git reset --soft HEAD~2
git commit --amend

Then once it looks good, git push -f updates this pull request.

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.

2 participants