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

{Makefile, ci}: Append .exe to our Windows binaries. #2120

Merged
merged 1 commit into from
Oct 10, 2023

Conversation

eiffel-fl
Copy link
Member

Suggested-by: Michael Friese mfriese@microsoft.com

@eiffel-fl eiffel-fl changed the title Makefile: Append .exe to our Windows binaries. {Makefile, ci}:: Append .exe to our Windows binaries. Oct 9, 2023
@eiffel-fl eiffel-fl changed the title {Makefile, ci}:: Append .exe to our Windows binaries. {Makefile, ci}: Append .exe to our Windows binaries. Oct 9, 2023
@mqasimsarfraz
Copy link
Member

Do you know if we will need changes here, https://github.com/inspektor-gadget/inspektor-gadget/blob/main/.krew.yaml#L67 ? I mean how the binary is called kubectl-gadget.exe

@eiffel-fl
Copy link
Member Author

Do you know if we will need changes here, https://github.com/inspektor-gadget/inspektor-gadget/blob/main/.krew.yaml#L67 ? I mean how the binary is called kubectl-gadget.exe

Good catch!
Adding such exception for such OS is really error prone.

@mqasimsarfraz
Copy link
Member

Adding such exception for such OS is really error prone.

True. It can be tricky. Hopefully we won't need any further changes 🤞

Also, I believe changes to .krew.yaml will mean manual merge in krew-index as well!

Suggested-by: Michael Friese <mfriese@microsoft.com>
Signed-off-by: Francis Laniel <flaniel@linux.microsoft.com>
Copy link
Member

@mqasimsarfraz mqasimsarfraz left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

I tested it locally and it worked fine:

→ ls -la ~/.krew/bin/kubectl-gadget.exe 
ls: cannot access '/home/qasim/.krew/bin/kubectl-gadget.exe': No such file or directory
→ cat gadget.yaml | grep 'os: windows' -A 5
        os: windows
        arch: amd64
    uri: https://github.com/inspektor-gadget/inspektor-gadget/releases/download/v0.21.0/kubectl-gadget-windows-amd64-v0.21.0.tar.gz
    sha256: 90c5dec8b4790b0f8e68a1e32373a450aef29d4c50f9f89c051c8dce498b87ec
    bin: kubectl-gadget.exe
→ KREW_OS=windows KREW_ARCH=amd64 kubectl-krew install --manifest=gadget.yaml --archive=kubectl-gadget-windows-amd64.tar.gz 
Installing plugin: gadget
Installed plugin: gadget
\
 | Use this plugin:
 | 	kubectl gadget
 | Documentation:
 | 	https://github.com/inspektor-gadget/inspektor-gadget
 | Caveats:
 | \
 |  | Inspektor Gadget needs to be deployed to each node:
 |  | 
 |  | $ kubectl gadget deploy
 |  | 
 |  | Read the documentation available at https://github.com/inspektor-gadget/inspektor-gadget
 |  | to get more information about the server side installation process.
 | /
/
→ ls -la ~/.krew/bin/kubectl-gadget.exe 
lrwxrwxrwx 1 qasim qasim 57 Okt 10 10:38 /home/qasim/.krew/bin/kubectl-gadget.exe -> /home/qasim/.krew/store/gadget/v0.21.0/kubectl-gadget.exe

@eiffel-fl eiffel-fl merged commit 47df93a into main Oct 10, 2023
48 of 49 checks passed
@eiffel-fl eiffel-fl deleted the francis/windows-exe branch October 10, 2023 12:12
@eiffel-fl
Copy link
Member Author

Thank you for the review!

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

2 participants