NO-JIRA: Normalize generation of driver metrics RBAC proxy sidecar#380
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@stephenfin: This pull request explicitly references no jira issue. DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
stephenfin
force-pushed
the
cleanup-driver-metrics
branch
from
9ebb32d to
4c9db2b
Compare
|
/retest |
stephenfin
force-pushed
the
cleanup-driver-metrics
branch
from
4c9db2b to
917c3be
Compare
openshift-ci
bot
requested review from
gnufied and
mandre
and removed request for
dobsonj
stephenfin
force-pushed
the
cleanup-driver-metrics
branch
from
917c3be to
2bfa3a1
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: stephenfin The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
|
Issues go stale after 90d of inactivity. Mark the issue as fresh by commenting If this issue is safe to close now please do so with /lifecycle stale |
openshift-ci
bot
added
the
lifecycle/stale
No need to use a slice. Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
stephenfin
force-pushed
the
cleanup-driver-metrics
branch
from
2bfa3a1 to
410d043
Compare
The 'InjectKubeRBACProxy' field was was always set to True in users and was therefore a no-op. The 'Name' field was likewise always set to 'driver-m' and doesn't need to be templated. Flatten the remaining two fields into the parent structs and remove MetricsPorts entirely. Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
stephenfin
force-pushed
the
cleanup-driver-metrics
branch
from
410d043 to
6764a2c
Compare
|
/remove-lifecycle stale |
openshift-ci
bot
removed
the
lifecycle/stale
|
/retest-required There's no obvious reason for those clusters not to have come up |
stephenfin
force-pushed
the
cleanup-driver-metrics
branch
from
6764a2c to
a4dcd7d
Compare
|
I've dropped the last commit since the rest of these changes are no-ops, as seen in the lack of changes to the assets. I'll propose that last commit separately. |
|
Well now we know it's unhappy CI(s): there are zero changes to the /retest-required |
|
/test aws-efs-operator-e2e |
|
/test aws-efs-operator-e2e |
|
/test e2e-openstack fwiw, these are happening because the cloud we are using has multiple CI systems (not just Prow) using it, and the other CI systems are stealing our resources. We are working overtime to get this resolved, but it involves multiple teams 😞 |
|
/lgtm There is no difference in the output of the generator, hence there is not much to test. |
openshift-ci-robot
added
the
verified
|
@jsafrane: This PR has been marked as verified by DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
|
/override "hypershift-e2e-openstack-aws-csi-cinder" We know this job is failing. We are working on the fix with openshift/hypershift#6909 and openshift/release#69631 but until then, as mentioned multiple times this doesn't affect generated assets and is a no-op from the production perspective. |
|
@stephenfin: /override requires failed status contexts, check run or a prowjob name to operate on.
Only the following failed contexts/checkruns were expected:
If you are trying to override a checkrun that has a space in it, you must put a double quote on the context. DetailsIn response to this:
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-sigs/prow repository. |
|
/override "ci/prow/hypershift-e2e-openstack-aws-csi-cinder" |
|
@stephenfin: Overrode contexts on behalf of stephenfin: ci/prow/hypershift-e2e-openstack-aws-csi-cinder DetailsIn response to this:
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-sigs/prow repository. |
|
/retest-required Oof |
|
/retest-required |
1 similar comment
|
/retest-required |
|
/hold Revision a4dcd7d was retested 3 times: holding |
openshift-ci
bot
added
the
do-not-merge/hold
|
/retest-required |
|
/hold cancel |
openshift-ci
bot
removed
the
do-not-merge/hold
|
@stephenfin: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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-sigs/prow repository. I understand the commands that are listed here. |
|
/retest-required |
openshift-merge-bot
bot
merged commit 4932cfe
into
openshift:main
5 participants
The generator has the ability to generate kube-rbac-proxy sidecar containers when exposing driver metrics on the controller or node.
In #379, we noted that while the sidecar containers templates used template variables, the actual driver template used hardcoded strings. This allows for the possibility of bugs if the port numbers are changed at any point.
Resolve this by modifying the generator such that the metric port for the drivers is now templated also. Changes are implemented in multiple steps: I'd encourage reviewers to look at the individual commits.
This includes the commits from #379 and that can as such be considered a dependency.That PR is now merged.Dependencies: