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

Fix setting PORTER_HOME on plugins #1235

Merged
merged 2 commits into from Aug 27, 2020

Conversation

carolynvs
Copy link
Member

What does this change

When running a plugin, I was setting PORTER_HOME improperly. Windows was having none of it and was refusing to run the plugin with an improperly formatted env var, returning

The parameter is incorrect

What issue does it fix

Closes #1154

Notes for the reviewer

As a follow-up I'm looking at https://docs.microsoft.com/en-us/azure/devops/pipelines/get-started-multiplatform?view=azure-devops to change our build to run on more than one OS. At first we can just run the CLI tests until we can build on all operating systems (not sure if building on windows is at 100% yet).

In the meantime, I've added a porter list call to our install script e2e test which does run on all OSs. This seems like a good thing to do anyway, make sure that Porter is not only installed but can run our most basic command without error.

Checklist

  • Unit Tests (e2e tests)
  • Documentation
  • Schema (porter.yaml)

When running a plugin, I was setting PORTER_HOME improperly. Windows was
having none of it and was refusing to run the plugin with an improperly
formatted env var, returning

The parameter is incorrect
@carolynvs carolynvs added this to In Progress in Porter and Mixins [OLD] via automation Aug 27, 2020
@carolynvs
Copy link
Member Author

/azp run porter-install-check

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Do a quick check that porter list works on all OSes during our install check
as a smoke test that what we installed works. This is a stopgap until we
can run our full test suite on all OSes in our PR CI run.
@carolynvs
Copy link
Member Author

/azp run porter-install-check

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@carolynvs
Copy link
Member Author

I triggered the porter-install-check build that normally only runs on main so that you can see how it will catch problems like this bug next time when there is a regression.

https://dev.azure.com/deislabs/porter/_build/results?buildId=6831&view=logs&j=2d2b3007-3c5c-5840-9bb0-2b1ea49925f3&t=c37637b8-ed57-5a02-c556-da2855a7e2d3

It is failing because it is testing canary and not this PRs build (that's the nature of that test). I expect it to pass when this PR is merged based on my manual testing on my local windows machine. It will need to be a separate effort to get our build running on a matrix of OSes for PR builds.

Screen Shot 2020-08-27 at 10 42 25 AM
Screen Shot 2020-08-27 at 10 43 28 AM

@carolynvs carolynvs marked this pull request as ready for review August 27, 2020 15:44
@carolynvs carolynvs merged commit dd9056f into getporter:main Aug 27, 2020
Porter and Mixins [OLD] automation moved this from In Progress to Done Aug 27, 2020
@carolynvs carolynvs deleted the fix-plugin-porterhome branch August 27, 2020 16:16
@vdice vdice removed this from Done in Porter and Mixins [OLD] Sep 16, 2020
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.

storage.porter.filesystem error on 0.27.1 with windows
2 participants