Skip to content
This repository has been archived by the owner on Nov 16, 2020. It is now read-only.

Add reason field to function to hold function creation errors #513

Merged
merged 2 commits into from
Jun 15, 2018

Conversation

rjew
Copy link
Contributor

@rjew rjew commented Jun 13, 2018

Fixes #483

  • Add reason field to function object
  • When an error occurs during the function image build phase, return the last step that occurred (the one that failed) along with its output as the reason
    • It's difficult to determine the exact cause of the error since Docker combines both stdout and stderr into one stream
    • The only error Docker gives is the last command that failed

Copy link
Contributor

@tenczar tenczar left a comment

Choose a reason for hiding this comment

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

Does this change populate the reason field when an error happens? It doesn't look like it would to me.

@rjew
Copy link
Contributor Author

rjew commented Jun 14, 2018

It does update the reason field. There was already a deferred call to h.Store.UpdateWithError in the function-manager controller.

@imikushin
Copy link
Contributor

An e2e-test of a failing function creation (e.g. with an invalid handler) would clearly demonstrate the reason field being populated

@rjew
Copy link
Contributor Author

rjew commented Jun 14, 2018

Good point. I'll add an e2e test.

Copy link
Contributor

@imikushin imikushin left a comment

Choose a reason for hiding this comment

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

Looks great!

Copy link
Contributor

@tenczar tenczar left a comment

Choose a reason for hiding this comment

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

cool

@imikushin imikushin merged commit 436c686 into vmware-archive:master Jun 15, 2018
@rjew rjew deleted the func-create-error branch June 15, 2018 16:01
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants