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

Panic in internal.(*ExecCommand).Start when -c flag is not specified #21

Closed
j-boivie opened this issue Oct 24, 2022 · 6 comments
Closed
Labels
bug Something isn't working

Comments

@j-boivie
Copy link

Hi!

I've been testing this tool but ran into an issue. When I run the tool I can navigate to a task that has ECS exec enabled, but when I select the container to exec into the program panics like this:

? Select a task: panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x1553488]

goroutine 6 [running]:
github.com/tedsmitt/ecsgo/internal.(*ExecCommand).executeInput(0xc00031caa0)
        github.com/tedsmitt/ecsgo/internal/exec.go:264 +0x88
github.com/tedsmitt/ecsgo/internal.(*ExecCommand).Start.func1()
        github.com/tedsmitt/ecsgo/internal/exec.go:72 +0x1e5
created by github.com/tedsmitt/ecsgo/internal.(*ExecCommand).Start
        github.com/tedsmitt/ecsgo/internal/exec.go:58 +0xee

I'm able to work around this by manually specifying the shell to run like ecsgo -c /bin/sh, which according to the README should be the default command.

@tedsmitt
Copy link
Owner

Hey @j-boivie thanks for raising this! I'm currently on vacation so will look at this when I get back next week 😄 In the meantime could you provide some details to help troubleshoot such as:

  • Number of containers in the task
  • Container names
  • Container image details

Thanks again!

@j-boivie
Copy link
Author

Hi,

Sorry for the late response, from my testing I've noticed that this happens on every task I connect to, no matter names, number of containers in the task etc. I've tested this across two different AWS accounts as well.

The only thing I can think of is that all of the ECS resources typically have hyphens in the name, and that they all run some variant of debian:-slim or python:-slim as base image.

Another thing I noticed is that the tool doesn't return a list of all services on the cluster, although I can see all tasks when selecting *. I'm only ever getting a list of 10 services, so I'm guessing that the reason is that this call here doesn't handle a paginated response from the ECS api:

ecsgo/internal/exec.go

Lines 123 to 126 in 95178ae

func (e *ExecCommand) getService() {
list, err := e.client.ListServices(&ecs.ListServicesInput{
Cluster: aws.String(e.cluster),
})

But I can raise another issue for that if needed.

@tedsmitt
Copy link
Owner

tedsmitt commented Nov 1, 2022

@j-boivie No problem! Thanks for the additional info.

Based on the error message it looks like this is the offending line:

if strings.Contains(*e.task.PlatformFamily, "Windows") {

Would I be right in assuming these tasks are running on an EC2-backed ECS cluster? I suspect that the PlatformFamily value may only be available by default in Fargate tasks, and might have to manually be specified for tasks with EC2 launch type. I have just tested this and I can recreate the problem. I'm writing up a fix now.

RE: Pagination you're absolutely right, I've not implemented this yet embarrassingly, it has been on my todo for too long 😓 If you could raise a new issue for this that'd be great!

tedsmitt pushed a commit that referenced this issue Nov 1, 2022
@tedsmitt tedsmitt added the bug Something isn't working label Nov 1, 2022
@j-boivie
Copy link
Author

j-boivie commented Nov 1, 2022

@j-boivie No problem! Thanks for the additional info.

Based on the error message it looks like this is the offending line:

if strings.Contains(*e.task.PlatformFamily, "Windows") {

Would I be right in assuming these tasks are running on an EC2-backed ECS cluster? I suspect that the PlatformFamily value may only be available by default in Fargate tasks, and might have to manually be specified for tasks with EC2 launch type. I have just tested this and I can recreate the problem. I'm writing up a fix now.

You're absolutely right, somehow I had forgotten that Fargate was a thing... 😅, but that seems reasonable.

RE: Pagination you're absolutely right, I've not implemented this yet embarrassingly, it has been on my todo for too long 😓 If you could raise a new issue for this that'd be great!

Roger that, will create a new issue 👍

tedsmitt pushed a commit that referenced this issue Nov 12, 2022
tedsmitt pushed a commit that referenced this issue Nov 12, 2022
tedsmitt pushed a commit that referenced this issue Nov 12, 2022
tedsmitt pushed a commit that referenced this issue Nov 12, 2022
tedsmitt pushed a commit that referenced this issue Nov 21, 2022
tedsmitt pushed a commit that referenced this issue Nov 21, 2022
@tedsmitt
Copy link
Owner

@j-boivie I've just pushed the fix for this in release 0.4.4, can you let me know if all looks good on your side? 😁

@j-boivie
Copy link
Author

@j-boivie I've just pushed the fix for this in release 0.4.4, can you let me know if all looks good on your side? 😁

All good here, thanks a lot for the fix! 🙇

tedsmitt pushed a commit that referenced this issue Jan 24, 2023
tedsmitt pushed a commit that referenced this issue Jan 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants