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

Add eksctl #374

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Add eksctl #374

wants to merge 5 commits into from

Conversation

mlopez122
Copy link

@mlopez122 mlopez122 commented Dec 10, 2021

What's Left to Do?

  • Create Cheat Sheet for eksctl

OP's Message

Following the provided instruction video I added the eksctl functionality to webinstall. I had some trouble in that I had to edit the ps1 file because when downloading the actual eksctl app, it comes as a single .exe file.
Additionally, My native operating system is Windows, but I was using an Ubuntu install to configure the files. As a result, I kept encountering a weird occurrence where each newline was terminated with a ^M character inside of the bash script. This kept throwing errors inside of my Ubuntu client. After some troubleshooting, I found per this stack overflow post that the issue was that ^M is the carriage-return character in Windows, and it can interfere with Linux/Mac behavior. After removing the character from the bash script, the tests ran normally per the video's testing instructions.

@mlopez122 mlopez122 marked this pull request as draft December 10, 2021 06:06
@mlopez122 mlopez122 marked this pull request as ready for review December 10, 2021 06:07
@mlopez122
Copy link
Author

resolves webinstall/webi-installer-requests#40

@coolaj86
Copy link
Member

Hey, just wanted to let you know that I appreciate this and I'll be taking a look at it within the next week or so.

I just spent a bunch of time on Webi PRs and updates a few weeks ago and I've needed to focus on some other projects for a bit to get caught up on them.

:)

title: eksctl
homepage: https://github.com/weaveworks/eksctl
tagline: |
The official CLI for Amazon EKS
Copy link
Member

Choose a reason for hiding this comment

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

Is this actually endorsed by Amazon? I think you may mean "original" or "best", but I'm pretty sure this isn't the "official" CLI. That's written in Python and described here: https://docs.aws.amazon.com/cli/latest/reference/eks/index.html

Or am I missing something?

The official CLI for Amazon EKS
---

To update or switch versions, run `webi example@stable` (or `@v2`, `@beta`,
Copy link
Member

Choose a reason for hiding this comment

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

Please update example text.


## Cheat Sheet

> From the `eksctl` ReadMe:
Copy link
Member

Choose a reason for hiding this comment

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

ReadMe should be stylized README.

```

### Add Baz Highlighting

Copy link
Member

Choose a reason for hiding this comment

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

Replace examples sections with real examples for eksctl.

Copy link
Member

@coolaj86 coolaj86 left a comment

Choose a reason for hiding this comment

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

I've requested a few changes. Please see inline comments.

Also, please run the code formatting tools before committing:

webi prettier
webi shfmt

# npx prettier@2 -w '**/*.{js,md,html}'
npm run prettier 

# shfmt -w -i 4 -sr -s ./
npm run shfmt

@coolaj86
Copy link
Member

This looks really good. Thanks so much for contributing.

I just have a few changes as seen above ^^. The main thing is that the docs need some fixing.

I'd love to have more cheat sheet examples, if you could. I don't use EKS, but since you do, you probably know the most common things that people want to do, and it would be great if you could add more of those. That said, it's NOT a blocking issue.

@coolaj86
Copy link
Member

P.S. This is up on beta: https://beta.webinstall.dev/eksctl

@coolaj86 coolaj86 force-pushed the add-eksctl branch 2 times, most recently from 46e08e3 to 2b10263 Compare January 28, 2022 09:17
@coolaj86 coolaj86 added documentation Improvements or additions to documentation first-timers-only good first issue Good for newcomers help wanted Extra attention is needed up-for-grabs labels Mar 5, 2022
@coolaj86
Copy link
Member

coolaj86 commented Mar 5, 2022

bump @mlopez122

@ivandev-81
Copy link

Is this issue already solved or not ?

@coolaj86
Copy link
Member

@navitux This was left half done. The Cheat Sheet still has stuff like example --foo --bar in it.

Could you round that out?

@coolaj86 coolaj86 removed documentation Improvements or additions to documentation good first issue Good for newcomers labels Nov 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants