fix(deadline): use new repository installer log path for Deadline 10.2.* #895
Conversation
There was a problem hiding this comment.
Great work, @AWS-Samuel!
Just two non-functional suggestions, so if you can incorporate them you shouldn't need to run integ tests again.
| /** | ||
| * The minimum Deadline version which uses Install Builder instead of Bitrock Installer. | ||
| */ | ||
| public static readonly MINIMUM_VERSION_USING_INSTALLBUILDER = new Version([10, 2, 0, 0]); |
There was a problem hiding this comment.
nit: Bitrock is the company that makes InstallBuilder. Both versions of Deadline use InstallBuilder to make installers, but I suspect that the version of InstallBuilder was changed between Deadline releases and that InstallBuilder changed its (default?) log file path.
Can we instead rename to MINIMUM_VERSION_USING_NEW_INSTALLBUILDER_LOG and adjust the code comment accordingly?
There was a problem hiding this comment.
Implemented in latest commit
| @@ -424,7 +424,9 @@ test('repository installer logs all required files', () => { | |||
| {}, // log group name. checked in another test. | |||
| '\",\"log_stream_name\":\"cloud-init-output-{instance_id}\",\"file_path\":\"/var/log/cloud-init-output.log\",\"timezone\":\"Local\"},{\"log_group_name\":\"', | |||
| {}, // log group name again. | |||
| '\",\"log_stream_name\":\"deadlineRepositoryInstallationLogs-{instance_id}\",\"file_path\":\"/tmp/bitrock_installer.log\",\"timezone\":\"Local\"}]}},\"log_stream_name\":\"DefaultLogStream-{instance_id}\",\"force_flush_interval\":15}}', | |||
| '\",\"log_stream_name\":\"deadlineRepositoryInstallationLogs-{instance_id}\",\"file_path\":\"/tmp/'+ | |||
| (version.isLessThan(Version.MINIMUM_VERSION_USING_INSTALLBUILDER) ? 'bitrock' : 'installbuilder') + | |||
There was a problem hiding this comment.
I don't think we actually triggering both code paths here, so there's some missed coverage.
I think the value comes from
There was a problem hiding this comment.
Good catch. I've done a bit of refactoring to support testing with multiple versions and can confirm that the test covers both cases now.
jusiskin
changed the title
jusiskin
changed the title
AWS-Samuel
force-pushed
the
samuel/fix-installer-log-location
branch
from
60a1627
to
28e34dd
Compare
Problem
With the release of Deadline
10.2.0.9, the deadline repository installer log file location changed from/tmp/bitrock_installer.logto/tmp/installbuilder_installer.log.This introduced a bug which made it so that the log stream in the repository component, meant to stream the repository installation logs, would no longer stream to cloudwatch.
Solution
Add a conditional check in the repository component which changes the path of the installer logs depending on the deadline version.
Testing
E2E Testing Details
Ran the end to end tests in the
integfolder by runningyarn e2ein theaws-rfdk/integdirectory.These end to end tests were run using both deadline
10.1.23.6(max version before installer log change) and10.2.0.9(min version after installer log change).When running for
10.1.23.6deadlineRepositoryInstallationLogs-<instance-id>cloud-init-output-<instance-id>deadlineRepositoryInstallationLogs-<instance-id>log stream was streaming installation logsWhen running for
10.2.0.9deadlineRepositoryInstallationLogs-<instance-id>cloud-init-output-<instance-id>deadlineRepositoryInstallationLogs-<instance-id>log stream was streaming installation logsBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license