-
Notifications
You must be signed in to change notification settings - Fork 493
Phase 1 changes for MCS Firstboot Pivot Failure Reporting #6029
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
base: main
Are you sure you want to change the base?
Changes from all commits
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 |
|---|---|---|
|
|
@@ -147,8 +147,14 @@ func buildSpec(dependencies *BootstrapDependencies, imgs *ctrlcommon.Images, rel | |
| templatectrl.DockerRegistryKey: imgs.DockerRegistry, | ||
| } | ||
|
|
||
| ignitionHost, err := getIgnitionHost(&dependencies.Infrastructure.Status) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| mcsURL := fmt.Sprintf("https://%s", ignitionHost) | ||
|
|
||
| config := getRenderConfig("", dependencies.KubeAPIServerServingCA, spec, | ||
| &imgs.RenderConfigImages, dependencies.Infrastructure, nil, nil, "2") | ||
| &imgs.RenderConfigImages, dependencies.Infrastructure, nil, nil, mcsURL, "2") | ||
|
Comment on lines
+150
to
+157
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. Guard Line 150 introduces a new bootstrap-time call path to Proposed hardening--- a/pkg/operator/sync.go
+++ b/pkg/operator/sync.go
@@
if infraStatus.PlatformStatus != nil {
switch infraStatus.PlatformStatus.Type {
case configv1.BareMetalPlatformType:
- ignitionHost = net.JoinHostPort(infraStatus.PlatformStatus.BareMetal.APIServerInternalIPs[0], securePortStr)
+ if infraStatus.PlatformStatus.BareMetal != nil && len(infraStatus.PlatformStatus.BareMetal.APIServerInternalIPs) > 0 {
+ ignitionHost = net.JoinHostPort(infraStatus.PlatformStatus.BareMetal.APIServerInternalIPs[0], securePortStr)
+ }
case configv1.OpenStackPlatformType:
- ignitionHost = net.JoinHostPort(infraStatus.PlatformStatus.OpenStack.APIServerInternalIPs[0], securePortStr)
+ if infraStatus.PlatformStatus.OpenStack != nil && len(infraStatus.PlatformStatus.OpenStack.APIServerInternalIPs) > 0 {
+ ignitionHost = net.JoinHostPort(infraStatus.PlatformStatus.OpenStack.APIServerInternalIPs[0], securePortStr)
+ }
case configv1.OvirtPlatformType:
- ignitionHost = net.JoinHostPort(infraStatus.PlatformStatus.Ovirt.APIServerInternalIPs[0], securePortStr)
+ if infraStatus.PlatformStatus.Ovirt != nil && len(infraStatus.PlatformStatus.Ovirt.APIServerInternalIPs) > 0 {
+ ignitionHost = net.JoinHostPort(infraStatus.PlatformStatus.Ovirt.APIServerInternalIPs[0], securePortStr)
+ }🤖 Prompt for AI Agents |
||
| return config, nil | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -45,10 +45,11 @@ type APIServer struct { | |||||||||||||||||||||||||||||
| // NewAPIServer initializes a new API server | ||||||||||||||||||||||||||||||
| // that runs the Machine Config Server as a | ||||||||||||||||||||||||||||||
| // handler. | ||||||||||||||||||||||||||||||
| func NewAPIServer(a *APIHandler, p int, is bool, c, k string, t *tls.Config) *APIServer { | ||||||||||||||||||||||||||||||
| func NewAPIServer(a *APIHandler, failureHandler *NodeFailureHandler, p int, is bool, c, k string, t *tls.Config) *APIServer { | ||||||||||||||||||||||||||||||
| mux := http.NewServeMux() | ||||||||||||||||||||||||||||||
| mux.Handle("/config/", a) | ||||||||||||||||||||||||||||||
| mux.Handle("/healthz", &healthHandler{}) | ||||||||||||||||||||||||||||||
| mux.Handle("/v1/node-failure", failureHandler) // NEW | ||||||||||||||||||||||||||||||
| mux.Handle("/", &defaultHandler{}) | ||||||||||||||||||||||||||||||
|
Comment on lines
+48
to
53
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. Guard against nil
Suggested fix func NewAPIServer(a *APIHandler, failureHandler *NodeFailureHandler, p int, is bool, c, k string, t *tls.Config) *APIServer {
mux := http.NewServeMux()
mux.Handle("/config/", a)
mux.Handle("/healthz", &healthHandler{})
- mux.Handle("/v1/node-failure", failureHandler) // NEW
+ if failureHandler != nil {
+ mux.Handle("/v1/node-failure", failureHandler)
+ }
mux.Handle("/", &defaultHandler{})📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| return &APIServer{ | ||||||||||||||||||||||||||||||
|
|
@@ -182,6 +183,71 @@ func (sh *APIHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { | |||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| type healthHandler struct{} | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| // NodeFailureHandler handles POST requests to /v1/node-failure | ||||||||||||||||||||||||||||||
| type NodeFailureHandler struct { | ||||||||||||||||||||||||||||||
| reporter FailureReporter | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| // NewNodeFailureHandler creates a handler for node failure reports | ||||||||||||||||||||||||||||||
| func NewNodeFailureHandler(reporter FailureReporter) *NodeFailureHandler { | ||||||||||||||||||||||||||||||
| return &NodeFailureHandler{ | ||||||||||||||||||||||||||||||
| reporter: reporter, | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| // ServeHTTP processes firstboot failure reports from nodes | ||||||||||||||||||||||||||||||
| func (h *NodeFailureHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { | ||||||||||||||||||||||||||||||
| // Only accept POST | ||||||||||||||||||||||||||||||
| if r.Method != http.MethodPost { | ||||||||||||||||||||||||||||||
| w.Header().Set("Content-Length", "0") | ||||||||||||||||||||||||||||||
| w.WriteHeader(http.StatusMethodNotAllowed) | ||||||||||||||||||||||||||||||
| return | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| // Read and decode JSON body | ||||||||||||||||||||||||||||||
| var report FirstbootFailureReport | ||||||||||||||||||||||||||||||
| decoder := json.NewDecoder(r.Body) | ||||||||||||||||||||||||||||||
| if err := decoder.Decode(&report); err != nil { | ||||||||||||||||||||||||||||||
| klog.Errorf("Failed to decode failure report: %v", err) | ||||||||||||||||||||||||||||||
| w.Header().Set("Content-Type", "application/json") | ||||||||||||||||||||||||||||||
| w.WriteHeader(http.StatusBadRequest) | ||||||||||||||||||||||||||||||
| json.NewEncoder(w).Encode(map[string]string{ | ||||||||||||||||||||||||||||||
| "error": "Invalid JSON payload", | ||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||
| return | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| // Validate required fields | ||||||||||||||||||||||||||||||
| if report.Pool == "" || report.NodeID == "" || report.Stage == "" { | ||||||||||||||||||||||||||||||
| klog.Errorf("Missing required fields in failure report: pool=%s node=%s stage=%s", | ||||||||||||||||||||||||||||||
| report.Pool, report.NodeID, report.Stage) | ||||||||||||||||||||||||||||||
| w.Header().Set("Content-Type", "application/json") | ||||||||||||||||||||||||||||||
| w.WriteHeader(http.StatusBadRequest) | ||||||||||||||||||||||||||||||
| json.NewEncoder(w).Encode(map[string]string{ | ||||||||||||||||||||||||||||||
| "error": "Missing required fields: pool, nodeID, and stage are required", | ||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||
| return | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| klog.Infof("Received failure report: pool=%s node=%s stage=%s", | ||||||||||||||||||||||||||||||
| report.Pool, report.NodeID, report.Stage) | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| // Report the failure (best-effort, errors are logged but don't fail the request) | ||||||||||||||||||||||||||||||
| ctx := r.Context() | ||||||||||||||||||||||||||||||
| if err := h.reporter.ReportFailure(ctx, &report); err != nil { | ||||||||||||||||||||||||||||||
| // Log error but still return 202 - we don't want the node to retry-loop | ||||||||||||||||||||||||||||||
| klog.Errorf("Failed to process failure report (still returning 202): %v", err) | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| // Always return 202 Accepted (best-effort delivery) | ||||||||||||||||||||||||||||||
| // Never return 5xx - the node must not retry-loop on reporter errors | ||||||||||||||||||||||||||||||
| w.Header().Set("Content-Type", "application/json") | ||||||||||||||||||||||||||||||
| w.WriteHeader(http.StatusAccepted) | ||||||||||||||||||||||||||||||
| json.NewEncoder(w).Encode(map[string]string{ | ||||||||||||||||||||||||||||||
| "status": "accepted", | ||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| type acceptHeaderValue struct { | ||||||||||||||||||||||||||||||
| MIMEType string | ||||||||||||||||||||||||||||||
| MIMESubtype string | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
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.
Exit after
NewClusterServererror to avoid nil dereference.When
NewClusterServerreturns an error, execution continues andcs.GetKubeClient()can panic. Fail fast after writing the termination error.Suggested fix
cs, err := server.NewClusterServer(startOpts.kubeconfig, startOpts.apiserverURL, startOpts.mcsURL) if err != nil { ctrlcommon.WriteTerminationError(err) + klog.Exitf("failed to initialize cluster server: %v", err) }Also applies to: 60-61
🤖 Prompt for AI Agents