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

feat(deadline): add ability to import repository settings #395

Merged
merged 2 commits into from
Apr 19, 2021

Conversation

jericht
Copy link
Contributor

@jericht jericht commented Apr 19, 2021

Partial fix for #397

Notes

Adds ability to import Deadline Repository settings with a repository settings file (see https://docs.thinkboxsoftware.com/products/deadline/10.1/1_User%20Manual/manual/repository-settings-importer-exporter.html)

Testing

Creating a JSON file that conformed to the repository settings importer format with a region and a path mapping rule. Modified the example app to use the file in the Repository construct and deployed it. Verified that:

  • The Repository installer group succeeds in importing the settings with the Repository installer.
  • Ran the ExportRepositorySettings Deadline command from the RenderQueue container and verified the settings I imported were in the output
    Also deployed without the settings and verified the installer still succeeds.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

@jericht jericht force-pushed the jericht/repository_configuration branch from b46c83d to 3079965 Compare April 19, 2021 17:23
@ddneilson ddneilson self-requested a review April 19, 2021 17:24
@ddneilson
Copy link
Contributor

General question... do we want to be doing this via Deadline's Repository installer? We have the ability to hit RCS endpoints in our Lambdas now, so we could do up a CustomResource that imports Repository settings.

I suppose the two are not mutually exclusive... it makes sense to have the ability to import settings to the repository during an initial installation of the repository, but it also may be desirable to be able to programmatically build out repository settings JSON from deployed infrastructure and import those settings post-Repository-install.

REPOSITORY_SETTINGS_ARG_STRING=''
if [ ! -z "$DEADLINE_REPOSITORY_SETTINGS_FILE" ]; then
if [ ! -f "$DEADLINE_REPOSITORY_SETTINGS_FILE" ]; then
echo "WARNING: Repository settings file was specified but is not a file: $DEADLINE_REPOSITORY_SETTINGS_FILE. Repository settings will not be imported."
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if we should make this case a fail. The warning's in the logs, but it looks like a successful deploy from the CDK CLI (and CloudFormation), but the customer would only know it's a fail if they look at the repository's cloud-init logs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a good point, changed this to fail the script

@horsmand horsmand self-requested a review April 19, 2021 17:32
Comment on lines 384 to 387
* The Deadline Repository settings file to import.
* @see https://docs.thinkboxsoftware.com/products/deadline/10.1/1_User%20Manual/manual/repository-settings-importer-exporter.html
*/
readonly repositorySettings?: Asset;
Copy link
Contributor

Choose a reason for hiding this comment

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

We should describe the default behaviour here too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added

@@ -887,7 +897,8 @@ export class Repository extends Construct implements IRepository {
private configureRepositoryInstallerScript(
installerGroup: AutoScalingGroup,
installPath: string,
version: IVersion) {
version: IVersion,
settings?: Asset) {
Copy link
Contributor

Choose a reason for hiding this comment

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

The closing parenthasis shoud be on a new line and the settings line should have a comma dangle.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@horsmand horsmand merged commit c55c078 into aws:mainline Apr 19, 2021
@jusiskin jusiskin added the contribution/core This is a PR that came from AWS. label Apr 19, 2021
@jusiskin jusiskin linked an issue May 3, 2021 that may be closed by this pull request
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contribution/core This is a PR that came from AWS.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Import Deadline Repository settings
4 participants