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

working on making knative/hack shell-free #299

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

sbdtu5498
Copy link

@sbdtu5498 sbdtu5498 commented Jul 10, 2023

Changes

/kind enhancement

Fixes #254

First of all, it is a hassle to maintain lots of shell scripts. Keeping them DRY is already difficult enough and they can't be used in the most efficient manner to automate the things up. Ultimately knative/hack by becoming shell free can be easily integrated with the CI/CD pipeline. It will make hack across knative and knative-sandbox org shell-free and will further increase developer productivity as the release can be automated easily via github-actions. Portability will be enhanced. Also, once the migration is complete, using a distroless image with few additional stuff such as Kubectl and git would be possible.

WARNING:-
This is a WIP. Please do not review or merge. Would be asking for review after few more iterations and it is quite a big task so would be taking some time and would be force pushing the changes from time to time. Thanks!

Release Note

NONE

Docs

NONE

Signed-off-by: Sanskar Bhushan <sbdtu5498@gmail.com>
@knative-prow
Copy link

knative-prow bot commented Jul 10, 2023

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: sbdtu5498
Once this PR has been reviewed and has the lgtm label, please assign vishal-chdhry for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@knative-prow knative-prow bot requested a review from upodroid July 10, 2023 03:12
@knative-prow
Copy link

knative-prow bot commented Jul 10, 2023

Welcome @sbdtu5498! It looks like this is your first PR to knative/hack 🎉

@knative-prow knative-prow bot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Jul 10, 2023
@knative-prow
Copy link

knative-prow bot commented Jul 10, 2023

Hi @sbdtu5498. Thanks for your PR.

I'm waiting for a knative member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@knative-prow knative-prow bot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Jul 10, 2023
@cardil
Copy link
Contributor

cardil commented Sep 1, 2023

Hi @sbdtu5498. It's wonderful you are doing this!

However, this PR is already far too large to ever be reviewed and approved. We need to work in smaller chunks.

Our current idea on #254 is to write specific, go-native tools in the https://github.com/knative/toolbox repository, and use them in current shell scripts. This allows to move the logic from scripts to proper tools in a gradual way.

I think you could partition the job you have already done here, into some tools, by creating PRs in toolbox repo.

WDYT?

/hold

@knative-prow knative-prow bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 1, 2023
@krsna-m
Copy link
Contributor

krsna-m commented Sep 1, 2023

Awesome work! Yea i agree this needs to be refactored into smaller pieces.

@sbdtu5498
Copy link
Author

@cardil @kvmware sorry for the delayed response. I will refactor this one and will put it in the listed repo. Thanks for letting me know.

@krsna-m
Copy link
Contributor

krsna-m commented Sep 19, 2023

@sbdtu5498 Thanks! Once you open the PR in toolbox please go ahead and let us know and close this PR.

@cardil
Copy link
Contributor

cardil commented Oct 10, 2023

Hey @sbdtu5498. I've updated the description of #254 with the approach we want to take. It also now tracks a number of tasks that are smaller in scope. The idea is to focus on a specific tool, write it in Go, and deploy it through the Knative repos. Hopefully, this will allow for a more incremental approach to this effort.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. kind/enhancement needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Hackless Go-native tools for Knative
3 participants