remove ability to pass in custom mustgather images #135
Conversation
openshift-ci
bot
added
the
approved
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #135 +/- ##
==========================================
+ Coverage 8.36% 10.67% +2.31%
==========================================
Files 4 4
Lines 275 281 +6
==========================================
+ Hits 23 30 +7
Misses 248 248
+ Partials 4 3 -1
|
|
/hold I haven't been able to test this yet with the proxy outage for backplane access, but the code changes should still be reviewable at this time. |
openshift-ci
bot
added
the
do-not-merge/hold
AlexVulaj
force-pushed
the
remove-mustgather-images
branch
2 times, most recently
from
99ab334 to
72ba0a3
Compare
|
/unhold |
openshift-ci
bot
removed
the
do-not-merge/hold
AlexVulaj
force-pushed
the
remove-mustgather-images
branch
2 times, most recently
from
f01b6a4 to
9ce97b6
Compare
There was a problem hiding this comment.
Looks a lot better, thanks for getting the version of the image that is the same as the cluster's version.
- Two requests around how we are getting the clusterversion that I think are important to address.
- I could be convinced around removing the existing
backOffLimitvalue and am just asking out of curiosity.
| return "", goerror.New("unable to determine cluster version from status history") | ||
| } | ||
|
|
||
| version = version[:strings.LastIndex(version, ".")] |
There was a problem hiding this comment.
Can we unit test this function? Small edge case, but I think this doesn't work when the version is something like 4.14.0-rc.0
You can use https://github.com/openshift/managed-cluster-validating-webhooks/blob/f0862cb7bf1869b00e1ab857ee08fcf6fc7f156c/pkg/webhooks/podimagespec/podimagespec_test.go as an example if you haven't setup a fake kube client before.
| func (r *MustGatherReconciler) getClusterVersionForJobTemplate(name string) (string, error) { | ||
| clusterVersion := &configv1.ClusterVersion{} | ||
| err := r.GetClient().Get(context.TODO(), types.NamespacedName{Name: name}, clusterVersion) | ||
| if err != nil { | ||
| return "", fmt.Errorf("unable to get clusterversion '%v': %w", name, err) | ||
| } | ||
|
|
||
| var version string | ||
| for _, history := range clusterVersion.Status.History { | ||
| if history.State == "Completed" { | ||
| version = history.Version | ||
| break | ||
| } | ||
| } | ||
| if version == "" { | ||
| return "", goerror.New("unable to determine cluster version from status history") | ||
| } | ||
|
|
||
| version = version[:strings.LastIndex(version, ".")] | ||
|
|
||
| return version, nil | ||
| } |
There was a problem hiding this comment.
Make sure to add RBAC so that the operator is able to get config.openshift.io/clusterversion objects https://github.com/openshift/must-gather-operator/blob/master/deploy/02_must-gather-operator.ClusterRole.yaml
AlexVulaj
force-pushed
the
remove-mustgather-images
branch
2 times, most recently
from
0251fbb to
50a7bc4
Compare
AlexVulaj
force-pushed
the
remove-mustgather-images
branch
from
254b7a6 to
b09d0ce
Compare
|
@AlexVulaj: all tests passed! Full PR test history. Your PR dashboard. 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. I understand the commands that are listed here. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: AlexVulaj, mjlshen 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 |
Follow up to OSD-21667 - we no longer need the ability to provide custom mustgather images.