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.

Sign up for GitHub

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

Jump to bottom

Better error message on ServiceAccountJSON decode failure #67

Merged
merged 1 commit into from
Feb 20, 2023
Merged

Better error message on ServiceAccountJSON decode failure #67

merged 1 commit into from
Feb 20, 2023

Conversation

AhmedMozaly
Copy link
Contributor

@AhmedMozaly AhmedMozaly commented Jan 17, 2023 •
edited
Loading

What this PR does / why we need it:
I spent more than an hour trying to troubleshoot invalid character '\n' in string literal when creating new VMs, At the end it was extra \n in the secret value

Thought it's better to guide the developer to the root cause of the error by enhancing the error message

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:

Release note:

Enhance error message when decoding serviceAccountJSON fails.

@AhmedMozaly AhmedMozaly requested review from a team as code owners January 17, 2023 12:07
@gardener-robot
Copy link

@AhmedMozaly Thank you for your contribution.

@gardener-robot gardener-robot added needs/review Needs review size/xs Size of pull request is tiny (see gardener-robot robot/bots/size.py) labels Jan 17, 2023
@CLAassistant
Copy link

CLAassistant commented Jan 17, 2023 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@gardener-robot-ci-3
Copy link
Contributor

Thank you @AhmedMozaly for your contribution. Before I can start building your PR, a member of the organization must set the required label(s) {'reviewed/ok-to-test'}. Once started, you can check the build status in the PR checks section below.

himanshu-kun
himanshu-kun approved these changes Jan 18, 2023
View reviewed changes
Copy link
Contributor

@himanshu-kun himanshu-kun left a comment

Choose a reason for hiding this comment

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

/lgtm
thanks for your changes

@gardener-robot gardener-robot added reviewed/lgtm Has approval for merging and removed needs/review Needs review labels Jan 18, 2023
@himanshu-kun himanshu-kun changed the title Better error message when decoding serviceAccountJSON fails Better error message on ServiceAccountJSON decode failure Jan 18, 2023
@himanshu-kun himanshu-kun added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jan 18, 2023
@gardener-robot-ci-1 gardener-robot-ci-1 added needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Jan 18, 2023
@AhmedMozaly
Copy link
Contributor Author

AhmedMozaly commented Jan 18, 2023 •
edited
Loading

@himanshu-kun I've a side question, posting here as I can't find other place
I've managed to install MCM into a k8s cluster, Could create machines and can see it created in the GCP console.

But it's always in "Pending" status when I run kubectl get machine I know this should be fixed by adding machine initialization script to the k8s secret that the MachineClass is referencing, But can't find an example initialization script to set.

Is there an example for what I should set in the k8s secret to allow the machine status to move from "Pending" to "Running"?

@rishabh-11 rishabh-11 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Jan 18, 2023
@gardener-robot-ci-1 gardener-robot-ci-1 added needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Jan 18, 2023
@himanshu-kun
Copy link
Contributor

hello @AhmedMozaly
we currently don't have an example initialization script which you can refer to. But I think its a major roadblock for anyone trying to get his hands dirty with MCM.
So we'll add a script in coming days. will create an issue for the same to track

@himanshu-kun
Copy link
Contributor

tracking issue -> gardener/machine-controller-manager#768

@AhmedMozaly
Copy link
Contributor Author

tracking issue -> gardener/machine-controller-manager#768

@himanshu-kun Thanks for the response, I've added a comment on the issue let's kindly continue discussion there

@rishabh-11 rishabh-11 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Jan 24, 2023
@gardener-robot-ci-2 gardener-robot-ci-2 added needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Jan 24, 2023
@rishabh-11 rishabh-11 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Jan 24, 2023
@gardener-robot-ci-1 gardener-robot-ci-1 added needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Jan 24, 2023
@himanshu-kun himanshu-kun self-assigned this Feb 9, 2023
@himanshu-kun himanshu-kun added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Feb 9, 2023
@gardener-robot-ci-3 gardener-robot-ci-3 added needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Feb 9, 2023
@himanshu-kun himanshu-kun added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Feb 17, 2023
@gardener-robot-ci-2 gardener-robot-ci-2 added needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Feb 17, 2023
@himanshu-kun himanshu-kun added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Feb 17, 2023
@gardener-robot-ci-3 gardener-robot-ci-3 added needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Feb 17, 2023
@gardener-robot-ci-3 gardener-robot-ci-3 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Feb 20, 2023
@himanshu-kun himanshu-kun merged commit a0fe36f into gardener:master Feb 20, 2023
@gardener-robot gardener-robot added the status/closed Issue is closed (either delivered or triaged) label Feb 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@himanshu-kun himanshu-kun himanshu-kun approved these changes

Assignees

@himanshu-kun himanshu-kun

Labels
needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) reviewed/lgtm Has approval for merging size/xs Size of pull request is tiny (see gardener-robot robot/bots/size.py) status/closed Issue is closed (either delivered or triaged)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@AhmedMozaly @gardener-robot @CLAassistant @gardener-robot-ci-3 @himanshu-kun @gardener-robot-ci-1 @gardener-robot-ci-2 @rishabh-11