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

Updated nginx Versions #63

Merged
merged 4 commits into from
Oct 14, 2024
Merged

Conversation

lefte
Copy link
Collaborator

@lefte lefte commented Oct 10, 2024

  • Added nginx version 1.27
  • Updated other bitnami versions
  • Minor improvements to curl commands

Bare minimum self-checks

What do you think of a person who only does the bare minimum?

  • I've updated this PR with the latest code from main
  • I've done a cursory QA pass of my code locally
  • I've ensured all automated status check and tests pass
  • I've connected this PR to an issue

Pieces of flare

  • I've written a unit or functional test for my code
  • I've updated relevant documentation it my code changes it
  • I've updated this repo's README if my code changes it
  • I've updated this repo's CHANGELOG with my change unless its a trivial change (like updating a typo in the docs)

Finally

If you have any issues or need help please join the #contributors channel in the Lando slack and someone will gladly help you out!

You can also check out the coder guide.

* Added nginx version 1.27
* Updated other bitnami versions
* Minor improvements to curl commands
Copy link

netlify bot commented Oct 10, 2024

Deploy Preview for lando-nginx failed. Why did it fail? →

Name Link
🔨 Latest commit e3be2f1
🔍 Latest deploy log https://app.netlify.com/sites/lando-nginx/deploys/6709c77493d4ae000807e8bb

@lefte
Copy link
Collaborator Author

lefte commented Oct 10, 2024

@reynoldsalec how do I add you as a reviewer? I feel like I might not have permissions to add you.

@reynoldsalec
Copy link
Member

Probably missing write permissions, I added you to the repo @lefte.

These failing tests are weird...they're supposed to fail (there is no https), but for some reason the error processing isn't working correctly. Definitely not related to anything you did AFAIK.

@AaronFeledy
Copy link
Member

It's failing at lando ssh -s defaults -c "curl https://localhost" || echo $? | grep 1 on all the tests because curl, as expected, returns error code 7 but the test is checking for error code 1. This worked in the past due to a bug in Lando that was swallowing exit codes instead of passing them along. That was fixed in the latest Lando release. The tests will need to be updated to check for exit code 7.

@lefte
Copy link
Collaborator Author

lefte commented Oct 10, 2024

It's failing at lando ssh -s defaults -c "curl https://localhost" || echo $? | grep 1 on all the tests because curl, as expected, returns error code 7 but the test is checking for error code 1. This worked in the past due to a bug in Lando that was swallowing exit codes instead of passing them along. That was fixed in the latest Lando release. The tests will need to be updated to check for exit code 7.

Makes sense, that was my thought as well, I'll make some changes to expect the right message result.

* Parse the exit code inside of the Leia test container
@lefte
Copy link
Collaborator Author

lefte commented Oct 11, 2024

Okay, I think I might have updated the 1.27 README to work in Leia, someone will have to trigger the test to run again, I don't have permission for that.

lefte added 2 commits October 11, 2024 00:26
* Attempt to repair the Leia test using another method
* Updated previous version tests to reflect Leia changes
@reynoldsalec reynoldsalec merged commit b8ebf17 into lando:main Oct 14, 2024
17 of 18 checks passed
@reynoldsalec
Copy link
Member

Looks great thanks @lefte for fighting the good fight here!

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.

3 participants