-
Notifications
You must be signed in to change notification settings - Fork 995
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
[Bug?]: Setting --verbose when deploying to baremetal "fails" during cleanup step. SshExecutor.js does not check that "args" is defined. #10654
Comments
@hjalmarhoglund Thanks for reporting this. I'm going to tag in another team member who specializes in deployment and will get back o you! |
…10663) **Problem** We recently improved the verbose output of the baremetal deployment process. It looks like this improvement has introduced a small bug. See #10654 for more details. **Changes** 1. Guard against `args` being undefined. --------- Co-authored-by: Tobbe Lundberg <tobbe@tlundberg.com>
…10663) **Problem** We recently improved the verbose output of the baremetal deployment process. It looks like this improvement has introduced a small bug. See #10654 for more details. **Changes** 1. Guard against `args` being undefined. --------- Co-authored-by: Tobbe Lundberg <tobbe@tlundberg.com>
…10663) **Problem** We recently improved the verbose output of the baremetal deployment process. It looks like this improvement has introduced a small bug. See #10654 for more details. **Changes** 1. Guard against `args` being undefined. --------- Co-authored-by: Tobbe Lundberg <tobbe@tlundberg.com>
Hey @hjalmarhoglund, would you mind trying out v7.6.1 and confirm that the fix works? |
Hey @Josh-Walker-GM! Sure, I've upgraded and tested deploying. I believe another error has been introduced. This time, the error affects both the verbose and the none-verbose baremetal deploys. Running,
gives the output
Running,
gives the output
The problemThe problem is that Looking at the new code for
which does not correctly set a space between the command and the first arg. Proposed fixI think this could be solved by changing line 13 to:
Testing this in a local
Gives the output
Testing if this solves deployingI try implementing my fix into
Hope this is of help, and thank you for getting to this as quickly as you did! Let me know if you want me to test anything again. |
Thanks for testing this so quickly! I agree we've messed up and introduced an even worse error! I'll get a fix and patch out again for this today |
Okay v7.6.2 is out with the further fix. Hopefully we didn't break it even more haha! |
Fantastic! I've tested deploying on v7.6.2. Both verbose and non-verbose works like a charm! |
What's not working?
When deploying to baremetal and setting the verbose flag,
all goes well until the last step, the cleanup step. The following output is produced:
Notably, the deploy doesn't actually fail. The app is re-deployed and everything works. If you deploy without the verbose flag,
no error occurs.
I believe that the error comes from
packages/cli/src/commands/deploy/baremetal/SshExecutor.js
, where on lines 21 and 31 no check ifargs
is undefined is done. I believe this is the case since the error only occurs when using the verbose flag.How do we reproduce the bug?
Run
yarn rw deploy baremetal production --verbose
towards your baremetal server.What's your environment? (If it applies)
Are you interested in working on this?
The text was updated successfully, but these errors were encountered: