Bug 2035239: Add pod listing permission to extract pod host IP #225

ardaguclu · 2021-12-23T10:58:37Z

To extract pod's host IP address, CBO tries to list pods in here

cluster-baremetal-operator/provisioning/baremetal_pod.go

Line 1071 in b7bba70

    
           podList, err := podClient.Pods(targetNamespace).List(context.Background(), listOptions)

However, CBO does not have allowance to list pods. This PR adds this.

ardaguclu · 2021-12-23T10:58:50Z

/test e2e-metal-ipi-virtualmedia

openshift-ci · 2021-12-23T11:03:14Z

@ardaguclu: This pull request references Bugzilla bug 2035239, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.10.0) matches configured target release for branch (4.10.0)
bug is in the state NEW, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

No GitHub users were found matching the public email listed for the QA contact in Bugzilla (augol@redhat.com), skipping review request.

Details

In response to this:

Bug 2035239: Add pod listing permission to extract pod host IP

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.

andfasano · 2021-12-23T13:04:06Z

/test e2e-metal-ipi-virtualmedia

ardaguclu · 2021-12-23T14:18:27Z

/test e2e-metal-ipi-virtualmedia

ardaguclu · 2021-12-23T14:26:08Z

provisioning/baremetal_pod.go

 func getPodHostIP(podClient coreclientv1.PodsGetter, targetNamespace string) (string, error) {
 	listOptions := metav1.ListOptions{
 		LabelSelector: metal3AppName,
-		FieldSelector: "status.hostIP",


I removed this because it failed with;

invalid selector: 'status.hostIP'; can't understand 'status.hostIP'" "name"="provisioning-configuration"

Maybe it is because of lack of get permission but I did this change for a quick fix

As an additional safety measure, can you please also verify that len(podList.Items) == 1 instead of > 0?

What do you think for the latest version @andfasano ?

Thanks, it seems ok, let's see the virtualmedia job

ardaguclu · 2021-12-23T14:53:38Z

/test e2e-metal-ipi-virtualmedia

ardaguclu · 2021-12-23T14:54:24Z

/test e2e-metal-ipi-virtualmedia

ardaguclu · 2021-12-23T16:04:27Z

/test e2e-metal-ipi-virtualmedia

ardaguclu · 2021-12-23T17:46:01Z

/test e2e-agnostic

ardaguclu · 2021-12-23T19:04:18Z

/test e2e-metal-ipi

ardaguclu · 2021-12-23T19:26:59Z

/test e2e-agnostic

ardaguclu · 2021-12-24T10:30:58Z

/test e2e-metal-ipi-virtualmedia

…ector

ardaguclu · 2021-12-24T11:34:56Z

/test e2e-metal-ipi-virtualmedia

openshift-ci · 2021-12-24T14:00:38Z

@ardaguclu: all tests passed!

Full PR test history. Your PR dashboard.

Details

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. I understand the commands that are listed here.

andfasano · 2021-12-24T14:19:07Z

/approve

openshift-ci · 2021-12-24T14:19:36Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: andfasano, ardaguclu

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [andfasano]

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

elfosardo · 2021-12-24T14:34:55Z

/lgtm

openshift-ci · 2021-12-24T14:36:14Z

@ardaguclu: All pull requests linked via external trackers have merged:

openshift/cluster-baremetal-operator#225

Bugzilla bug 2035239 has been moved to the MODIFIED state.

Details

In response to this:

Bug 2035239: Add pod listing permission to extract pod host IP

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.

Even if there is no metal3 Pod up and therefore we cannot find its IP address, we still want to create the image-customization-controller with an empty ironic IP address. This is documented in a comment at the site where getPodHostIP() is called, but not in the implementation, so it was inadvertently broken by openshift#225.

openshift-ci bot requested review from andfasano and sadasu December 23, 2021 10:59

ardaguclu changed the title ~~Add pod listing permission to extract pod host IP~~ Bug 2035239: Add pod listing permission to extract pod host IP Dec 23, 2021

openshift-ci bot added bugzilla/severity-high Referenced Bugzilla bug's severity is high for the branch this PR is targeting. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. labels Dec 23, 2021

ardaguclu force-pushed the add-pods-list-rbac branch from 91b217f to ad9832a Compare December 23, 2021 14:17

ardaguclu commented Dec 23, 2021

View reviewed changes

ardaguclu force-pushed the add-pods-list-rbac branch from ad9832a to cb1311a Compare December 23, 2021 14:52

ardaguclu force-pushed the add-pods-list-rbac branch from cb1311a to 5fd6ad6 Compare December 23, 2021 16:03

ardaguclu force-pushed the add-pods-list-rbac branch from 5fd6ad6 to 2f28af5 Compare December 24, 2021 10:29

Add pod listing permission to extract pod host IP and remove fieldSel…

34c3d92

…ector

ardaguclu force-pushed the add-pods-list-rbac branch from 2f28af5 to 34c3d92 Compare December 24, 2021 11:22

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 24, 2021

openshift-ci bot assigned elfosardo Dec 24, 2021

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Dec 24, 2021

openshift-merge-robot merged commit 7f9a818 into openshift:master Dec 24, 2021

zaneb mentioned this pull request Jan 4, 2022

Bug 2037483: Allow CBO to list Pods only in the openshift-machine-api namespace #227

Merged

zaneb mentioned this pull request Jan 7, 2022

Bug 2038272: Create image-customization-controller when metal3 Pod not up #231

Merged

ardaguclu deleted the add-pods-list-rbac branch January 25, 2022 08:29

Bug 2035239: Add pod listing permission to extract pod host IP #225

Bug 2035239: Add pod listing permission to extract pod host IP #225

Uh oh!

Conversation

ardaguclu commented Dec 23, 2021

Uh oh!

ardaguclu commented Dec 23, 2021

Uh oh!

openshift-ci bot commented Dec 23, 2021

Uh oh!

andfasano commented Dec 23, 2021

Uh oh!

ardaguclu commented Dec 23, 2021

Uh oh!

ardaguclu Dec 23, 2021

Choose a reason for hiding this comment

Uh oh!

andfasano Dec 24, 2021

Choose a reason for hiding this comment

Uh oh!

ardaguclu Dec 24, 2021

Choose a reason for hiding this comment

Uh oh!

andfasano Dec 24, 2021

Choose a reason for hiding this comment

Uh oh!

ardaguclu commented Dec 23, 2021

Uh oh!

ardaguclu commented Dec 23, 2021

Uh oh!

ardaguclu commented Dec 23, 2021

Uh oh!

ardaguclu commented Dec 23, 2021

Uh oh!

ardaguclu commented Dec 23, 2021

Uh oh!

ardaguclu commented Dec 23, 2021

Uh oh!

ardaguclu commented Dec 24, 2021

Uh oh!

ardaguclu commented Dec 24, 2021

Uh oh!

openshift-ci bot commented Dec 24, 2021

Uh oh!

andfasano commented Dec 24, 2021

Uh oh!

openshift-ci bot commented Dec 24, 2021

Uh oh!

elfosardo commented Dec 24, 2021

Uh oh!

openshift-ci bot commented Dec 24, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants