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

Consolidate service files #2067

Merged
merged 8 commits into from
Dec 1, 2016

Conversation

matthewrmshin
Copy link
Contributor

@matthewrmshin matthewrmshin commented Nov 16, 2016

  1. Create service (variable) files under ~/cylc-run/SUITE/.service/.
    • source - symbolic link to suite source (can be ..)
      • Was path=SOURCE in ~/.cylc/REGDB/SUITE.
      • There is no need to store suite title. Read title from suite.rc is very fast.
    • ssl.pem - SSL private key file
      • Was in SOURCE/
    • ssl.cert - SSL certificate file
      • Was in SOURCE/
    • passphrase - passphrase file
      • Was in SOURCE/
    • contact - suite contact file
      • Was ~/cylc-run/SUITE/cylc-suite-env and ~/.cylc/ports/SUITE.
    • db - private suite run SQLite database file
      • Was ~/cylc-run/SUITE/cylc-suite-private.db
  2. Suite contact file absorbs the port file.
    • Don't bother installing the contact file on job hosts that require polling.
    • Skip contact file installation for job host with shared file system with suite host.
    • On suite stop, remove suite contact files, local + remote ones.
    • Task message will no longer retry if suite contact file not found, as suite is unlikely to be running, and restart will poll any way.
  3. Hierarchical suite name delimiter is now the path separator /.
    • Was .
    • It should now be natural to have suites in different levels of sub-directories under ~/cylc-run/
  4. Remove subcommands: copy, unregister, rereregister, refresh.
    • (Happy to reinstate if this is a concern.)
  5. cylc run|restart: automatic register, if necessary.
    • New --source=SOURCE option to specify alternate suite source.
  6. cylc run|restart: (if previous run has left behind a contact file) remove old contact file and proceed with suite start up if it can prove that the old suite process is no longer running.
  7. suite.rc.processed generation:
    • cylc validate: will no longer generate suite.rc.processed by default. New --output=FILENAME option to specify location to dump processed suite.rc file.
    • cylc run: will now dump suite.rc.processed under suite run directory, instead of the suite source directory.
  8. Public suite run SQLite database file relocated to log/db, but will continue to have a symbolic link from top of suite run directory as cylc-suite.db.
  9. Group suites of each test battery run under ~/cylc-run/cylctb-TIMESTAMP/ where TIMESTAMP is the start time of the test battery run. The hierarchy under which would also roughly reflect the hierarchy under $CYLC_DIR/tests/.

Close #2054. (Note: There are some minor changes from the original proposal.)

@cylc/core Change should be in a good state for review, but please discuss any concerns you may have. I expect #2063 to conflict with this change, so I think #2063 should probably go in before this.

@matthewrmshin matthewrmshin added this to the next release milestone Nov 16, 2016
@matthewrmshin matthewrmshin self-assigned this Nov 16, 2016
matthewrmshin added a commit to matthewrmshin/rose that referenced this pull request Nov 16, 2016
@matthewrmshin matthewrmshin force-pushed the consolidate-service-files branch 4 times, most recently from 998e04c to 4ce0e92 Compare November 21, 2016 11:48
matthewrmshin added a commit to matthewrmshin/rose that referenced this pull request Nov 21, 2016
@hjoliver
Copy link
Member

@matthewrmshin - I'm hoping to have a good look at this before the end of the week.

@matthewrmshin
Copy link
Contributor Author

Branch re-based and conflicts resolved.

@arjclark
Copy link
Contributor

@matthewrmshin - this is on my to-do list - haven't forgotten just need space to examine and consider.

@hjoliver
Copy link
Member

Very nice change.

A couple of minor questions on file naming:

  • .cylc-var - what's the meaning of "variable" here? what about just .service?
  • the "cylc" in the public db name cylc-suite.db is kind of redundant too - should it be just db like the new private db name? (could keep the top level symlink the same for back-compatibility)

@hjoliver
Copy link
Member

Removing the old db category commands is fine with me.

Copy link
Member

@hjoliver hjoliver left a comment

Choose a reason for hiding this comment

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

Looks good; approved regardless of potential file name changes as per my question above.

@matthewrmshin
Copy link
Contributor Author

Hi @hjoliver:

  • On .cylc-var... At first, I proposed .cylc-service, and then I was trying to find a name with a meaning that roughly agrees with FHS. On hindsight, files in this directory are not going to change continuously during run time, so var is not really suitable either. Happy to change to .service.
  • On log/cylc-suite.db... Make sense. I'll modify it to log/db.

matthewrmshin added a commit to matthewrmshin/rose that referenced this pull request Nov 25, 2016
matthewrmshin added a commit to matthewrmshin/rose that referenced this pull request Nov 25, 2016
Remove redundant items.
Support `/` in suite names.
matthewrmshin added a commit to matthewrmshin/rose that referenced this pull request Nov 28, 2016
Remove redundant items.
Support `/` in suite names.
matthewrmshin added a commit to matthewrmshin/rose that referenced this pull request Nov 28, 2016
Remove redundant items.
Support `/` in suite names.
matthewrmshin added a commit to matthewrmshin/rose that referenced this pull request Nov 29, 2016
Remove redundant items.
Support `/` in suite names.

[skip ci]
1. Create service files under ~/cylc-run/SUITE/.cylc-var/.
   * source - symbolic link to suite source (can be `..`)
   * ssl.pem - SSL private key file
   * ssl.cert - SSL certificate file
   * passphrase - passphrase file
   * contact - suite contact file
   * db - private suite run SQLite database file
2. Suite contact file absorbs the port file.
   * On suite stop, remove suite contact files, local + remotes.
   * Task message will no longer retry if suite contact file not found,
     as suite is unlikely to be running, and restart will poll any way.
3. Hierarchical suite name delimiter is now the path separator "/".
4. Remove subcommands: copy, unregister, rereregister, refresh.
5. cylc run|restart: automatic register, if necessary.
   * New `--source=SOURCE` option to specify alternate suite source.
6. Generate `suite.rc.processed` only on demand:
   * cylc validate: new `--output=FILENAME` option to specify location
     to dump processed `suite.rc` file.
   * cylc run: dump `suite.rc.processed` under suite run directory.
7. Public suite run SQLite database file relocated to `log/cylc-suite.db`,
   but will continue to have a symbolic link from top of suite run
   directory.
Now that suites are easily identifiable under `~/cylc-run/`, it makes
sense to group suites of each test battery run under
`~/cylc-run/cylctb-TIMESTAMP/` where TIMESTAMP is the start time of the
test battery run. The hierarchy under which would also roughly reflect
the hierarchy under `$CYLC_DIR/tests/`.
On suite start up, if an old contact file is found, read host and
process string from the contact file and run a (ssh+)ps command to
determine if an old suite is still running or not. If it can confirm
that the old suite process does not exist, it will remove the old
contact file and continue with starting up the suite.  Otherwise, it
will print the old error message.
If `task communication method=poll`, don't bother installing  the
contact file on the job host.

Also remove old back-compat for "remote shell template".
Detect presence of contact file before attempt to register suite.
* `.service/` <= `.cylc-var/`
* `log/db` <= `log/cylc-suite.db`
Don't sync service files if remote job host has shared file system with
suite host.
@arjclark
Copy link
Contributor

arjclark commented Dec 1, 2016

Can't spot anything obviously wrong or missing. Think I preferred the old cylc reg way of doing things but understand not necessarily needed. Test battery passing for me.

@arjclark arjclark merged commit c7e2317 into cylc:master Dec 1, 2016
@matthewrmshin matthewrmshin deleted the consolidate-service-files branch December 1, 2016 14:57
matthewrmshin added a commit to matthewrmshin/rose that referenced this pull request Dec 1, 2016
Remove redundant items.
Support `/` in suite names.
matthewrmshin added a commit to matthewrmshin/rose that referenced this pull request Dec 1, 2016
Remove redundant items.
Support `/` in suite names.
arjclark added a commit to metomi/rose that referenced this pull request Dec 8, 2016
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.

4 participants