-
Notifications
You must be signed in to change notification settings - Fork 13
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
Add pkg/log
package and cleanup logging around the codebase
#544
Changes from 6 commits
78f7514
c20bdbb
d34c74a
b6053cb
831be6c
7d42115
f281429
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,7 +3,6 @@ package impl | |
import ( | ||
"context" | ||
"fmt" | ||
"log" | ||
|
||
apiv1 "github.com/canonical/k8s/api/v1" | ||
"github.com/canonical/k8s/pkg/snap" | ||
|
@@ -46,22 +45,15 @@ func GetLocalNodeStatus(ctx context.Context, s *state.State, snap snap.Snap) (ap | |
if err != nil { | ||
return apiv1.NodeStatus{}, fmt.Errorf("failed to check if node is a worker: %w", err) | ||
} | ||
|
||
if isWorker { | ||
clusterRole = apiv1.ClusterRoleWorker | ||
} else { | ||
node, err := nodeutil.GetControlPlaneNode(ctx, s, s.Name()) | ||
if err != nil { | ||
// The node is likely in a joining or leaving phase where the role is not yet settled. | ||
// Use the unknown role but still log this incident for debugging. | ||
log.Printf("Failed to check if node is control-plane. This is expected if the node is in a joining/leaving phase. %v", err) | ||
clusterRole = apiv1.ClusterRoleUnknown | ||
} else { | ||
if node != nil { | ||
return *node, nil | ||
} | ||
} | ||
|
||
} else if node, err := nodeutil.GetControlPlaneNode(ctx, s, s.Name()); err != nil { | ||
clusterRole = apiv1.ClusterRoleUnknown | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is the log here no longer necessary? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes, removed log lines that did not provide any useful context here and in some other places There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @bschimke95 is this useful to keep around? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would keep around the comment on when this might happen. Let's remove the logs though. |
||
} else if node != nil { | ||
return *node, nil | ||
} | ||
|
||
return apiv1.NodeStatus{ | ||
Name: s.Name(), | ||
Address: s.Address().Hostname(), | ||
|
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.
Neat!