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

Configure linting for scripts/ directory #14

Closed
wants to merge 2 commits into from

Conversation

Jasstkn
Copy link
Member

@Jasstkn Jasstkn commented Jun 18, 2022

Signed-off-by: Jasstkn [email protected]

What this PR does / why we need it:
Add GA workflow to lint scripts

Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged): fixes #13

Special notes for your reviewer:

Checklist:

  • Fixes #
  • PR messages has document related information
  • Labelled this PR & related issue with breaking-changes tag
  • PR messages has breaking changes related information
  • Labelled this PR & related issue with requires-upgrade tag
  • PR messages has upgrade related information
  • Commit has unit tests
  • Commit has integration tests
  • E2E run Required for the changes

@Jasstkn Jasstkn requested a review from neelanjan00 as a code owner June 18, 2022 12:31
@Jasstkn Jasstkn force-pushed the configure-scripts-lint branch from 8b25de1 to fb8805a Compare June 18, 2022 12:32
@Jasstkn
Copy link
Member Author

Jasstkn commented Jun 18, 2022

@neelanjan00 hi. I wonder if I need to fix issues that have been found by shellcheck in this PR?

@neelanjan00 neelanjan00 added enhancement New feature or request good first issue Good for newcomers labels Jun 18, 2022
@neelanjan00
Copy link
Member

Hi @Jasstkn, thank you so much for the PR! 🙂 It will surely be great if you can resolve the issues in the shell script(s) as well since the CI is failing i.e. the shell scripts indeed require fixes. After the changes, you can also test your changes by installing/uninstalling m-agent in any Linux-based OS and the build script for building the binaries.

@Jasstkn Jasstkn changed the title Configure linting for scripts/ directory Draft: Configure linting for scripts/ directory Jun 19, 2022
@Jasstkn Jasstkn force-pushed the configure-scripts-lint branch 3 times, most recently from 933b3fb to 901f3d4 Compare June 19, 2022 06:20
@Jasstkn Jasstkn force-pushed the configure-scripts-lint branch from 901f3d4 to ebe7bbb Compare June 19, 2022 06:23
@Jasstkn
Copy link
Member Author

Jasstkn commented Jun 19, 2022

Hi @Jasstkn, thank you so much for the PR! 🙂 It will surely be great if you can resolve the issues in the shell script(s) as well since the CI is failing i.e. the shell scripts indeed require fixes. After the changes, you can also test your changes by installing/uninstalling m-agent in any Linux-based OS and the build script for building the binaries.

Hey. Looks like on my mac m1 there are some issue with running this script. I've tried to run in CI but it fails because of missing go environment variables. Do you have any ideas how we can handled it easily? For now I just increased the severity level for which shellcheck should fail.

@Jasstkn Jasstkn changed the title Draft: Configure linting for scripts/ directory Configure linting for scripts/ directory Jun 19, 2022
@neelanjan00
Copy link
Member

neelanjan00 commented Jun 19, 2022

Hi @Jasstkn, thank you so much for the PR! 🙂 It will surely be great if you can resolve the issues in the shell script(s) as well since the CI is failing i.e. the shell scripts indeed require fixes. After the changes, you can also test your changes by installing/uninstalling m-agent in any Linux-based OS and the build script for building the binaries.

Hey. Looks like on my mac m1 there are some issue with running this script. I've tried to run in CI but it fails because of missing go environment variables. Do you have any ideas how we can handled it easily? For now I just increased the severity level for which shellcheck should fail.

The shell script checks for the OS and CPU arch before the actual installation. Currently, only Linux is supported for OS but MacOS is Unix aka. Darwin hence it isn't supported. You can use a Linux VM to run the script for testing.

@Jasstkn
Copy link
Member Author

Jasstkn commented Jun 19, 2022

Hi @Jasstkn, thank you so much for the PR! 🙂 It will surely be great if you can resolve the issues in the shell script(s) as well since the CI is failing i.e. the shell scripts indeed require fixes. After the changes, you can also test your changes by installing/uninstalling m-agent in any Linux-based OS and the build script for building the binaries.

Hey. Looks like on my mac m1 there are some issue with running this script. I've tried to run in CI but it fails because of missing go environment variables. Do you have any ideas how we can handled it easily? For now I just increased the severity level for which shellcheck should fail.

The shell script checks for the OS and CPU arch before the actual installation. Currently, only Linux is supported for OS but MacOS is Unix aka. Darwin hence it isn't supported. You can use a Linux VM to run the script for testing.

Then what are the pre-requisites? Go installed? And could you elaborate a bit how it should be configured? Because I'm getting errors about badly configured GOROOT and etc.

@Jasstkn
Copy link
Member Author

Jasstkn commented Jun 20, 2022

@neelanjan00 ping

@neelanjan00
Copy link
Member

neelanjan00 commented Jun 21, 2022

Hi @Jasstkn, thank you so much for the PR! 🙂 It will surely be great if you can resolve the issues in the shell script(s) as well since the CI is failing i.e. the shell scripts indeed require fixes. After the changes, you can also test your changes by installing/uninstalling m-agent in any Linux-based OS and the build script for building the binaries.

Hey. Looks like on my mac m1 there are some issue with running this script. I've tried to run in CI but it fails because of missing go environment variables. Do you have any ideas how we can handled it easily? For now I just increased the severity level for which shellcheck should fail.

The shell script checks for the OS and CPU arch before the actual installation. Currently, only Linux is supported for OS but MacOS is Unix aka. Darwin hence it isn't supported. You can use a Linux VM to run the script for testing.

Then what are the pre-requisites? Go installed? And could you elaborate a bit how it should be configured? Because I'm getting errors about badly configured GOROOT and etc.

Hi @Jasstkn, sorry for the late reply. To run the m-agent program, you need to have Go installed. Also, there's a file that you need to create in /etc/m-agent by the name of PORT, which will contain the port at which m-agent server would listen. Can you check the Go version? We have developed using the 1.17 version.

Also, can you please mention the exact steps to reproduce the error and attach a screenshot of the error here?

@Jasstkn
Copy link
Member Author

Jasstkn commented Jun 26, 2022

Hi @Jasstkn, thank you so much for the PR! 🙂 It will surely be great if you can resolve the issues in the shell script(s) as well since the CI is failing i.e. the shell scripts indeed require fixes. After the changes, you can also test your changes by installing/uninstalling m-agent in any Linux-based OS and the build script for building the binaries.

Hey. Looks like on my mac m1 there are some issue with running this script. I've tried to run in CI but it fails because of missing go environment variables. Do you have any ideas how we can handled it easily? For now I just increased the severity level for which shellcheck should fail.

The shell script checks for the OS and CPU arch before the actual installation. Currently, only Linux is supported for OS but MacOS is Unix aka. Darwin hence it isn't supported. You can use a Linux VM to run the script for testing.

Then what are the pre-requisites? Go installed? And could you elaborate a bit how it should be configured? Because I'm getting errors about badly configured GOROOT and etc.

Hi @Jasstkn, sorry for the late reply. To run the m-agent program, you need to have Go installed. Also, there's a file that you need to create in /etc/m-agent by the name of PORT, which will contain the port at which m-agent server would listen. Can you check the Go version? We have developed using the 1.17 version.

Also, can you please mention the exact steps to reproduce the error and attach a screenshot of the error here?

$ go version
go version go1.18.1 linux/amd64

Error is the following:

$ ./m-agent/scripts/build.sh m-agent
Building linux-386
package m-agent: no Go files in /usr/lib/go-1.18/src/m-agent
An error has occurred with exit code 0! Aborting the script execution...

@Jasstkn
Copy link
Member Author

Jasstkn commented Jun 30, 2022

@neelanjan00 ping

@neelanjan00
Copy link
Member

Hi @Jasstkn, sorry for the late reply. I am not sure why you're getting this error, when I run sudo bash ./install.sh I get the following message in my Macbook Pro with M1 Pro chip, which is the expected UX:
Screenshot 2022-06-30 at 11 42 58 PM

@Jasstkn
Copy link
Member Author

Jasstkn commented Jun 30, 2022

Hi @Jasstkn, sorry for the late reply. I am not sure why you're getting this error, when I run sudo bash ./install.sh I get the following message in my Macbook Pro with M1 Pro chip, which is the expected UX: Screenshot 2022-06-30 at 11 42 58 PM

As you mentioned - I need to have a linux machine. So, I used lima-vm project to achieve this. The output above is from ubuntu virtual machine that I spined up.

@neelanjan00
Copy link
Member

Can you try to run the command that I used above?

@Jasstkn
Copy link
Member Author

Jasstkn commented Jul 2, 2022

sudo bash ./install.sh

ran successfully.

lima@lima-ubuntu:/Users/maria.kotlyarevskaya/go/pkg/mod/github.com/litmuschaos/m-agent/scripts$ sudo bash ./install.sh
./install.sh: line 63: netstat: command not found
Downloading https://m-agent.s3.amazonaws.com/m-agent-linux-amd64-2.10.0-Beta1.tar.gz
Preparing to install m-agent into /usr/local/bin
m-agent installed into /usr/local/bin/m-agent
m-agent server PORT config file created
Downloading https://m-agent.s3.amazonaws.com/m-agent.service
Setting up m-agent into /etc/systemd/system
Created symlink /etc/systemd/system/multi-user.target.wants/m-agent.service → /etc/systemd/system/m-agent.service.

The current issue is related to build script.

@neelanjan00
Copy link
Member

neelanjan00 commented Jul 4, 2022

sudo bash ./install.sh

ran successfully.

lima@lima-ubuntu:/Users/maria.kotlyarevskaya/go/pkg/mod/github.com/litmuschaos/m-agent/scripts$ sudo bash ./install.sh
./install.sh: line 63: netstat: command not found
Downloading https://m-agent.s3.amazonaws.com/m-agent-linux-amd64-2.10.0-Beta1.tar.gz
Preparing to install m-agent into /usr/local/bin
m-agent installed into /usr/local/bin/m-agent
m-agent server PORT config file created
Downloading https://m-agent.s3.amazonaws.com/m-agent.service
Setting up m-agent into /etc/systemd/system
Created symlink /etc/systemd/system/multi-user.target.wants/m-agent.service → /etc/systemd/system/m-agent.service.

The current issue is related to build script.

Got it. Actually, the build script expects ENVs and CLI params such as the package path, tag, target OS, target Architecture, etc. When configured, it builds the m-agent binary for an array of CPU architectures for Linux OS. A more straightforward way to build the binary for testing on your Linux VM is to directly build the code with go build.

Assuming your VM is using an amd64 CPU architecture:

env GOOS=linux GOARCH=amd64 go build cmd/m-agent/main.go

@Jasstkn
Copy link
Member Author

Jasstkn commented Jul 16, 2022

sudo bash ./install.sh

ran successfully.

lima@lima-ubuntu:/Users/maria.kotlyarevskaya/go/pkg/mod/github.com/litmuschaos/m-agent/scripts$ sudo bash ./install.sh
./install.sh: line 63: netstat: command not found
Downloading https://m-agent.s3.amazonaws.com/m-agent-linux-amd64-2.10.0-Beta1.tar.gz
Preparing to install m-agent into /usr/local/bin
m-agent installed into /usr/local/bin/m-agent
m-agent server PORT config file created
Downloading https://m-agent.s3.amazonaws.com/m-agent.service
Setting up m-agent into /etc/systemd/system
Created symlink /etc/systemd/system/multi-user.target.wants/m-agent.service → /etc/systemd/system/m-agent.service.

The current issue is related to build script.

Got it. Actually, the build script expects ENVs and CLI params such as the package path, tag, target OS, target Architecture, etc. When configured, it builds the m-agent binary for an array of CPU architectures for Linux OS. A more straightforward way to build the binary for testing on your Linux VM is to directly build the code with go build.

Assuming your VM is using an amd64 CPU architecture:

env GOOS=linux GOARCH=amd64 go build cmd/m-agent/main.go

I just wanted to verify that script is working properly after my changes. Maybe you can handle this part?

@Jasstkn
Copy link
Member Author

Jasstkn commented Aug 1, 2022

@neelanjan00 ping

@Jasstkn Jasstkn closed this Feb 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Shell Lint to Build and Push Pipelines
2 participants