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
Bug 2002262: Fix user-agent in vCenter sessions list #912
Bug 2002262: Fix user-agent in vCenter sessions list #912
Conversation
@@ -67,6 +67,7 @@ func GetOrCreate( | |||
return &session, nil | |||
} | |||
} | |||
klog.Infof("Create new vcenter session") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this definitely something we want? If so, I'd suggest
klog.Infof("Create new vcenter session") | |
klog.Infof("No existing vCenter session found, creating new session") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should appear just once during controller startup. IMHO would be nice to have such message.
Updated message
For some reason vCenter using user-agent from first login request and does not respect it's change after. Moved login step into our code with setting user-agent for soap client beforehand. Info log message for new vCenter session creation added by the way.
676ab1b
to
4601d06
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this generally makes sense to me
/lgtm
@lobziik: The following tests failed, say
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/test-infra repository. I understand the commands that are listed here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/approve
thanks
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: alexander-demichev 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 |
/retest-required Please review the full test history for this PR and help us cut down flakes. |
1 similar comment
/retest-required Please review the full test history for this PR and help us cut down flakes. |
Is it worth creating a BZ for this and backporting it to 4.9/4.8? |
/retest-required Please review the full test history for this PR and help us cut down flakes. |
Yeah, worth to backport. Will do. |
@lobziik: All pull requests linked via external trackers have merged: Bugzilla bug 2002262 has been moved to the MODIFIED state. In 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/test-infra repository. |
/bugzilla-refresh |
/cherry-pick release-4.9 |
@JoelSpeed: new pull request created: #918 In 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/test-infra repository. |
For some reason vCenter using user-agent from first
login request and does not respect it's change after.
Moved login step into our code with setting user-agent
for soap client beforehand.
Info log message for new vCenter session creation added
by the way.
tested on vCenter 7.0.1.00200