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

🐛 Return error on infra cluster and control plane discovery #8604

Merged
merged 1 commit into from
May 4, 2023
Merged

🐛 Return error on infra cluster and control plane discovery #8604

merged 1 commit into from
May 4, 2023

Conversation

tobiasgiese
Copy link
Member

If no control plane reference is set in the Cluster object the describe cluster command will fail. We should validate the error in any case.

What this PR does / why we need it:

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #8603

Tobias Giese tobias.giese@mercedes-benz.com, Mercedes-Benz Tech Innovation GmbH, legal info/Impressum

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label May 4, 2023
@k8s-ci-robot k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. area/clusterctl Issues or PRs related to clusterctl labels May 4, 2023
@tobiasgiese tobiasgiese force-pushed the tobiasgiese/fix-clusterctl-describe-cluster-nil-pointer branch from 719ca46 to fabd2f4 Compare May 4, 2023 12:20
@tobiasgiese tobiasgiese changed the title Return error on infra cluster and control plane discovery 🐛 \Return error on infra cluster and control plane discovery May 4, 2023
@tobiasgiese tobiasgiese changed the title 🐛 \Return error on infra cluster and control plane discovery 🐛 Return error on infra cluster and control plane discovery May 4, 2023
@tobiasgiese tobiasgiese mentioned this pull request May 4, 2023
Closed
@tobiasgiese tobiasgiese force-pushed the tobiasgiese/fix-clusterctl-describe-cluster-nil-pointer branch from fabd2f4 to 14c022b Compare May 4, 2023 16:20
@tobiasgiese
Return error on infra cluster and control plane discovery
f928305 
If no control plane reference is set in the Cluster object the describe cluster
command will fail. We should validate the error in any case.

Signed-off-by: Tobias Giese <tobias.giese@mercedes-benz.com>
@tobiasgiese tobiasgiese force-pushed the tobiasgiese/fix-clusterctl-describe-cluster-nil-pointer branch from 14c022b to f928305 Compare May 4, 2023 16:33
fabriziopandini
fabriziopandini reviewed May 4, 2023
View reviewed changes
Copy link
Member

@fabriziopandini fabriziopandini left a comment

Choose a reason for hiding this comment

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

/lgtm

@ykakarap and @Jont828 PTAL

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 4, 2023
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: 4498cacec88468e5c1a39fcc20d293b9e0ea7314

ykakarap
ykakarap approved these changes May 4, 2023
View reviewed changes
Copy link
Contributor

@ykakarap ykakarap left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ykakarap

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

The pull request process is described 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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 4, 2023
@ykakarap
Copy link
Contributor

ykakarap commented May 4, 2023

Probably worth back-porting this fix into the supported release branches?

@tobiasgiese
Copy link
Member Author

/hold cancel

@k8s-ci-robot k8s-ci-robot merged commit ce1ea02 into kubernetes-sigs:main May 4, 2023
@k8s-ci-robot k8s-ci-robot added this to the v1.5 milestone May 4, 2023
@tobiasgiese tobiasgiese deleted the tobiasgiese/fix-clusterctl-describe-cluster-nil-pointer branch May 4, 2023 22:51
@killianmuldoon
Copy link
Contributor

/cherry-pick release-1.4

@killianmuldoon
Copy link
Contributor

/cherry-pick release-1.3

@k8s-infra-cherrypick-robot k8s-infra-cherrypick-robot mentioned this pull request May 4, 2023
Merged
@k8s-infra-cherrypick-robot
Copy link

@killianmuldoon: new pull request created: #8609

In response to this:

/cherry-pick release-1.4

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.

@k8s-infra-cherrypick-robot
Copy link

@killianmuldoon: new pull request created: #8610

In response to this:

/cherry-pick release-1.3

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.

@k8s-infra-cherrypick-robot k8s-infra-cherrypick-robot mentioned this pull request May 4, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fabriziopandini fabriziopandini fabriziopandini left review comments

@ykakarap ykakarap ykakarap approved these changes

@Jont828 Jont828 Awaiting requested review from Jont828

@stmcginnis stmcginnis Awaiting requested review from stmcginnis

Assignees

@fabriziopandini fabriziopandini

@ykakarap ykakarap

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/clusterctl Issues or PRs related to clusterctl cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Milestone
v1.5
Development

Successfully merging this pull request may close these issues.

clusterctl fails with a nil pointer if no controlPlaneRef set in Cluster
6 participants
@tobiasgiese @k8s-ci-robot @ykakarap @killianmuldoon @k8s-infra-cherrypick-robot @fabriziopandini