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.

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

Fix incorrect rolebinding error message and improve logging #2564

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

jrvaldes
Copy link
Contributor

This PR introduces fixes and improvements to the logging messages in both the cmd and controller packages to enhance debugging capabilities. Particularly, correct the log message for RB deletion to show the namespace of the malformed resource properly.

This commit introduces a log to print the platform type of
the cluster to enhance the WMCO pod log for better triage.
This commit improve the WICD roleBinding and clusterRoleBinding creation
log messages with addition information for better correlation.
This commit adjust the roleBinding creation log message to
correctly show the namespace of the malformed resource
existing in the cluster.
@jrvaldes
Copy link
Contributor Author

jrvaldes commented Dec 2, 2024

/test remaining-required

meta.CreateOptions{})
if err != nil {
return fmt.Errorf("unable to create RoleBinding %s/%s: %w", r.watchNamespace, wicdRBACResourceName, err)
}
r.log.Info("Created resource", "RoleBinding",
kubeTypes.NamespacedName{Namespace: r.watchNamespace, Name: expectedRB.Name})
kubeTypes.NamespacedName{Namespace: createdRB.Namespace, Name: createdRB.Name},
Copy link
Member

Choose a reason for hiding this comment

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

wouldn't this be same as expectedRB?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, with namespaced resources isn't a big deal. I made this change to better align with the ClusterRoleBinding log messages.

@jrvaldes
Copy link
Contributor Author

jrvaldes commented Dec 5, 2024

/test aws-e2e-operator

@jrvaldes
Copy link
Contributor Author

jrvaldes commented Dec 6, 2024

/retest-required

Copy link
Contributor

openshift-ci bot commented Dec 9, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jrvaldes, sebsoto

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

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 9, 2024
@jrvaldes
Copy link
Contributor Author

jrvaldes commented Dec 9, 2024

vsphere-e2e-operator job failed but wmco test suite passed

https://prow.ci.openshift.org/view/gs/test-platform-results/pr-logs/pull/openshift_windows-machine-config-operator/2564/pull-ci-openshift-windows-machine-config-operator-master-vsphere-e2e-operator/1865070189909905408#1:build-log.txt%3A69

@openshift/openshift-team-windows-containers Please consider overriding the failed jobs

@mansikulkarni96
Copy link
Member

/lgtm

@mansikulkarni96
Copy link
Member

/override ci/prow/vsphere-e2e-operator ci/prow/vsphere-disconnected-e2e-operator

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Dec 10, 2024
Copy link
Contributor

openshift-ci bot commented Dec 10, 2024

@mansikulkarni96: Overrode contexts on behalf of mansikulkarni96: ci/prow/vsphere-disconnected-e2e-operator, ci/prow/vsphere-e2e-operator

In response to this:

/override ci/prow/vsphere-e2e-operator ci/prow/vsphere-disconnected-e2e-operator

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.

@openshift-ci-robot
Copy link

/retest-required

Remaining retests: 0 against base HEAD 84de66e and 2 for PR HEAD a739d43 in total

@jrvaldes
Copy link
Contributor Author

/test azure-e2e-operator

timed out

@jrvaldes
Copy link
Contributor Author

/test azure-e2e-operator

to gather more data

@openshift-ci-robot
Copy link

/retest-required

Remaining retests: 0 against base HEAD e8f8083 and 2 for PR HEAD a739d43 in total

3 similar comments
@openshift-ci-robot
Copy link

/retest-required

Remaining retests: 0 against base HEAD e8f8083 and 2 for PR HEAD a739d43 in total

@openshift-ci-robot
Copy link

/retest-required

Remaining retests: 0 against base HEAD e8f8083 and 2 for PR HEAD a739d43 in total

@openshift-ci-robot
Copy link

/retest-required

Remaining retests: 0 against base HEAD e8f8083 and 2 for PR HEAD a739d43 in total

@openshift-ci-robot
Copy link

/retest-required

Remaining retests: 0 against base HEAD c8b270e and 1 for PR HEAD a739d43 in total

@openshift-ci-robot
Copy link

/retest-required

Remaining retests: 0 against base HEAD c8b270e and 2 for PR HEAD a739d43 in total

@jrvaldes
Copy link
Contributor Author

/test azure-e2e-operator

@openshift-ci-robot
Copy link

/retest-required

Remaining retests: 0 against base HEAD 2e679d8 and 1 for PR HEAD a739d43 in total

@openshift-ci-robot
Copy link

/retest-required

Remaining retests: 0 against base HEAD 2e679d8 and 2 for PR HEAD a739d43 in total

@openshift-ci-robot
Copy link

/retest-required

Remaining retests: 0 against base HEAD 2e679d8 and 2 for PR HEAD a739d43 in total

5 similar comments
@openshift-ci-robot
Copy link

/retest-required

Remaining retests: 0 against base HEAD 2e679d8 and 2 for PR HEAD a739d43 in total

@openshift-ci-robot
Copy link

/retest-required

Remaining retests: 0 against base HEAD 2e679d8 and 2 for PR HEAD a739d43 in total

@openshift-ci-robot
Copy link

/retest-required

Remaining retests: 0 against base HEAD 2e679d8 and 2 for PR HEAD a739d43 in total

@openshift-ci-robot
Copy link

/retest-required

Remaining retests: 0 against base HEAD 2e679d8 and 2 for PR HEAD a739d43 in total

@openshift-ci-robot
Copy link

/retest-required

Remaining retests: 0 against base HEAD 2e679d8 and 2 for PR HEAD a739d43 in total

@openshift-ci-robot
Copy link

/retest-required

Remaining retests: 0 against base HEAD 6c32252 and 1 for PR HEAD a739d43 in total

@jrvaldes
Copy link
Contributor Author

/test azure-e2e-operator

@jrvaldes
Copy link
Contributor Author

/test vsphere-e2e-operator

@openshift-ci-robot
Copy link

/retest-required

Remaining retests: 0 against base HEAD 25e4d53 and 0 for PR HEAD a739d43 in total

@openshift-ci-robot
Copy link

/hold

Revision a739d43 was retested 3 times: holding

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 20, 2024
@jrvaldes
Copy link
Contributor Author

/test azure-e2e-operator

@jrvaldes
Copy link
Contributor Author

/hold cancel

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 23, 2024
@openshift-ci-robot
Copy link

/retest-required

Remaining retests: 0 against base HEAD 25e4d53 and 2 for PR HEAD a739d43 in total

@jrvaldes
Copy link
Contributor Author

/retest-required

Copy link
Contributor

openshift-ci bot commented Dec 24, 2024

@jrvaldes: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/vsphere-e2e-operator a739d43 link true /test vsphere-e2e-operator

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants