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

[POA-1513] Add support for different agent install paths in EC2 add #28

Merged
merged 4 commits into from
Aug 12, 2024

Conversation

mudit-postman
Copy link
Contributor

In this PR, we have changed the ec2 setup command to check for the agent install path before setting it in the postman-insight-agent systemd file.

Changes done:

  • Use FS template to write agent path in postman-insights-agent.service file
  • A new util function to check for the eligible install path.
  • Moved FS Template logic to a separate util funciton
  • Other refactoring stuff

@mudit-postman mudit-postman requested a review from mgritter August 12, 2024 06:36
@mudit-postman mudit-postman marked this pull request as ready for review August 12, 2024 06:36
Copy link
Contributor

@mgritter mgritter left a comment

Choose a reason for hiding this comment

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

Please fix the typo; otherwise looks OK.

reportStep(message)

for _, path := range agentInstallPaths {
if _, err := exec.LookPath(path); err == nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder whether we should be using LookPath directly, for just "postman-insights-agent", and using whichever is first in the path? Failing that, maybe use argv[0]?

I don't think there's anything necessarily wrong with building in the set of paths that we know the install script uses today, but it does mean that the two have to be kept in sync. So that's why I'm wondering whether we could be more flexible in terms of "wherever in the path postman-insights-agent" is installed or "wherever you successfully ran the agent" -- the latter seems very reasonable to me if we can make it work?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have added a check for the default agent path also.

}

envFile, err := os.Create(envFilePath)
err = util.GenrateAndWriteTemplateFile(envFileFS, envFileTemplateName, envFileBasePath, envFileName, envFiledata)
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo

Suggested change
err = util.GenrateAndWriteTemplateFile(envFileFS, envFileTemplateName, envFileBasePath, envFileName, envFiledata)
err = util.GenerateAndWriteTemplateFile(envFileFS, envFileTemplateName, envFileBasePath, envFileName, envFiledata)

@mudit-postman mudit-postman requested a review from mgritter August 12, 2024 16:15
@mudit-postman mudit-postman merged commit e48d3d6 into main Aug 12, 2024
4 checks passed
@mudit-postman mudit-postman deleted the mudit/POA-1513 branch August 12, 2024 17:15
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.

2 participants