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

Little improvements in etcd_bootstrap_script.sh #28

Closed
wants to merge 1 commit into from
Closed

Little improvements in etcd_bootstrap_script.sh #28

wants to merge 1 commit into from

Conversation

StenlyTU
Copy link

What this PR does / why we need it:
Improve Indentation and readability.

Special notes for your reviewer:

Release note:

NONE

@StenlyTU StenlyTU requested a review from a team as a code owner December 19, 2022 08:35
@gardener-robot gardener-robot added needs/review Needs review size/m Size of pull request is medium (see gardener-robot robot/bots/size.py) labels Dec 19, 2022
@shreyas-s-rao shreyas-s-rao self-assigned this Dec 19, 2022
Copy link
Contributor

@shreyas-s-rao shreyas-s-rao left a comment

Choose a reason for hiding this comment

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

@StenlyTU thanks for your PR. I tested it locally and saw that etcd container throws the following error log:

standard_init_linux.go:228: exec user process caused: exec format error

Can you please check this? Thanks.

@gardener-robot gardener-robot added the needs/changes Needs (more) changes label Dec 19, 2022
echo $RET > $VALIDATION_MARKER
exit $RET
arch=$(uname -m)
if [ $arch = "aarch64" ] || [ $arch = "arm64" ]; then

Choose a reason for hiding this comment

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

Can you also enclose $arch in double quotes? This is usually done to prevent gobbling and word splitting.

"Successful")
echo "Bootstrap preprocessing end time: $(date)"
start_managed_etcd
break ;;

Choose a reason for hiding this comment

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

This is unreachable line and should ideally be removed. Call to start-managed-etcd function has exit as the last statement which means break will never be called.

@unmarshall
Copy link

@StenlyTU thanks for your PR. I tested it locally and saw that etcd container throws the following error log:

standard_init_linux.go:228: exec user process caused: exec format error

Can you please check this? Thanks.

I think you have ARM based chipset. Can you check if this is not an issue with using the wrong image on built for another architecture?

@shreyas-s-rao
Copy link
Contributor

@StenlyTU we haven't heard anything from you from a while. We have started work on #16, which would invalidate any changes you've made in this PR. So I would vote in favour of closing this PR since the changes made here don't bring any functional improvement, for the cost of making a new release of the etcd-custom-image for the same etcd version being used within gardener, given that we will be restructuring the repo, code and releases.
Are you ok with this suggestion?

@StenlyTU StenlyTU closed this Jan 6, 2023
@gardener-robot gardener-robot added the status/closed Issue is closed (either delivered or triaged) label Jan 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs/changes Needs (more) changes needs/review Needs review size/m Size of pull request is medium (see gardener-robot robot/bots/size.py) status/closed Issue is closed (either delivered or triaged)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants