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: Initialize VCSClient for commandrunner to remove recent server panics #3461

Merged
merged 3 commits into from
May 30, 2023

Conversation

pseudomorph
Copy link
Contributor

@pseudomorph pseudomorph commented May 30, 2023

what

VCSClient was inadvertently removed from the commandrunner instantiation in #3086.

https://github.com/runatlantis/atlantis/pull/3086/files#diff-78f42ba40d0f10b08c73b7e6bb8376f398e249c963cf549e89591d6b6826b9a4L780

This has caused server panics during the course of normal operations.

why

Bugfix.

tests

Added a unit test to ensure that VCSClient is passed to commandRunner during server initialization.

references

@pseudomorph pseudomorph requested a review from a team as a code owner May 30, 2023 14:02
@github-actions github-actions bot added the go Pull requests that update Go code label May 30, 2023
nitrocode
nitrocode previously approved these changes May 30, 2023
@nitrocode
Copy link
Member

@pseudomorph can you describe how you've tested this in the pr body? And/or is there a way to add a unit test or e2e test for this to prevent this from happening in the future?

@nitrocode nitrocode changed the title Initialize VCSClient for commandrunner. fix: Initialize VCSClient for commandrunner to remove recent server panics May 30, 2023
@pseudomorph
Copy link
Contributor Author

Let me look into an e2e test.

@pseudomorph
Copy link
Contributor Author

pseudomorph commented May 30, 2023

e2e test initializes the server struct directly, not through the NewServer function. Added a unit test instead to ensure that VCSClient is passed in to commandRunner during server initialization.

@GenPage GenPage merged commit ad75aac into runatlantis:main May 30, 2023
mtavaresmedeiros pushed a commit to mtavaresmedeiros/atlantis that referenced this pull request Jul 3, 2023
…anics (runatlantis#3461)

* Initialize VCSClient for commandrunner.

* Add test.
ijames-gc pushed a commit to gocardless/atlantis that referenced this pull request Feb 13, 2024
…anics (runatlantis#3461)

* Initialize VCSClient for commandrunner.

* Add test.
ijames-gc pushed a commit to gocardless/atlantis that referenced this pull request Feb 13, 2024
…anics (runatlantis#3461)

* Initialize VCSClient for commandrunner.

* Add test.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
go Pull requests that update Go code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Panic encountered after upgrading from 0.22 to 0.24
4 participants