Skip to content
This repository has been archived by the owner on Oct 24, 2023. It is now read-only.

fix: get-logs command collects control plane logs even if apiserver is down #3939

Merged
merged 7 commits into from
Oct 21, 2020

Conversation

haofan-ms
Copy link
Contributor

@haofan-ms haofan-ms commented Oct 15, 2020

Reason for Change:

When api server is down, the kubeClient.ListNodes() function will not be able to get nodes. In this case we want to get control panel nodes using other configurations and collect logs of these nodes.

Issue Fixed:

Credit Where Due:

Does this change contain code from or inspired by another project?

  • No
  • Yes

If "Yes," did you notify that project's maintainers and provide attribution?

  • No
  • Yes

Requirements:

Notes:

@jackfrancis
Copy link
Member

to @jadarsie for final approval/merge

@jadarsie
Copy link
Member

to @jadarsie for final approval/merge

checking

cmd/get_logs.go Outdated
@@ -362,6 +365,17 @@ func (glc *getLogsCmd) getCloudName() string {
return ""
}

func (glc *getLogsCmd) getControlPanelNodes() []v1.Node {
Copy link
Member

Choose a reason for hiding this comment

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

computeControlPanelNodes may be slightly better

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated

cmd/get_logs.go Outdated
@@ -362,6 +365,17 @@ func (glc *getLogsCmd) getCloudName() string {
return ""
}

func (glc *getLogsCmd) getControlPanelNodes() []v1.Node {
Copy link
Member

Choose a reason for hiding this comment

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

Could you please add a test for this method?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

test added

cmd/get_logs.go Outdated
@@ -362,6 +365,17 @@ func (glc *getLogsCmd) getCloudName() string {
return ""
}

func (glc *getLogsCmd) getControlPanelNodes() []v1.Node {
var ControlPanelNodeList []v1.Node
Copy link
Member

@jadarsie jadarsie Oct 16, 2020

Choose a reason for hiding this comment

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

shorter camel-cased local variable names are more idiomatic in my opinion

ex: i instead of nodeIndex or node instead of controlPlaneNode

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

@acs-bot acs-bot added size/M and removed size/S labels Oct 16, 2020
@jadarsie jadarsie changed the title fix: Get-logs for control panel nodes when api server is down fix: get-logs command collects control plane logs even if apiserver is down Oct 21, 2020
@jadarsie jadarsie merged commit 9d0bb9c into Azure:master Oct 21, 2020
@acs-bot
Copy link

acs-bot commented Oct 21, 2020

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: haofan-ms, jadarsie

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

penggu pushed a commit to penggu/aks-engine that referenced this pull request Oct 28, 2020
…s down (Azure#3939)

* fix: get-logs command collects control plane logs even if apiserver is down

* update

* update message

* update

* update format

* update typos
@haofan-ms haofan-ms deleted the get-logs-control-panel branch February 16, 2022 04:33
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants