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 not finding netclient binary (introduced in #613) #642

Closed
wants to merge 1 commit into from

Conversation

krombel
Copy link

@krombel krombel commented Nov 29, 2023

Describe your changes

fix finding binary in docker startup script

Provide testing steps

Start docker container (without this fix) so you see, that it cannot find the netlink binary so it is not working at all.
With this fix the binary ist found again.

Checklist before requesting a review

  • My changes affect only 10 files or less.
  • I have performed a self-review of my code and tested it.
  • If it is a new feature, I have added thorough tests, my code is <= 1450 lines.
  • If it is a bugfix, my code is <= 200 lines.
  • My functions are <= 80 lines.
  • I have had my code reviewed by a peer.
  • My unit tests pass locally.
  • Netclient & Netmaker are awesome.

Copy link
Member

Choose a reason for hiding this comment

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

docker build places the binary in /root folder
I am not sure in which scenario binary would be missing...

Copy link
Author

@krombel krombel Apr 9, 2024

Choose a reason for hiding this comment

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

The issue is, that we call netclient via /root/netclient and via netclient directly (which would require /root to be in $PATH which is not the case)

Copy link
Member

Choose a reason for hiding this comment

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

COPY --from=builder /app/netclient-app ./netclient

netclient binary is copied to /root and then installed via startup script

Copy link
Author

Choose a reason for hiding this comment

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

netclient install is using /root/ (so not needing this patch) but netclient join relies on PATH and is failing currently

Copy link
Member

Choose a reason for hiding this comment

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

netclient install will put the binary in /bin path, so full path reference is not required

@krombel krombel force-pushed the docker_fix_binary_not_found branch 2 times, most recently from 63509ec to b52be88 Compare April 10, 2024 15:19
@krombel krombel force-pushed the docker_fix_binary_not_found branch from b52be88 to 317f528 Compare April 11, 2024 13:03
@abhishek9686
Copy link
Member

closing this PR as these changes are not required

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.

None yet

3 participants