Skip to content

Commit b39f4af

Browse files
NicholasYanceyNicholasYancey
authored
Merge pull request #17 from migtools/nicholasyancey
Resolving Bugs with describe.go and logs.go
2 parents 101cb01 + f21a1b2 commit b39f4af

File tree

3 files changed

+119
-55
lines changed

3 files changed

+119
-55
lines changed

‎cmd/non-admin/backup/create.go‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -366,6 +366,9 @@ func (o *CreateOptions) BuildNonAdminBackup(namespace string) (*nacv1alpha1.NonA
366366
} else {
367367
// Build the BackupSpec manually
368368
// For NonAdminBackup, automatically include the current namespace
369+
370+
storageLocation := o.StorageLocation
371+
369372
backupBuilder := builder.ForBackup(namespace, o.Name).
370373
IncludedNamespaces(namespace). // Automatically include the current namespace
371374
IncludedResources(o.IncludeResources...).
@@ -377,7 +380,7 @@ func (o *CreateOptions) BuildNonAdminBackup(namespace string) (*nacv1alpha1.NonA
377380
LabelSelector(o.Selector.LabelSelector).
378381
OrLabelSelector(o.OrSelector.OrLabelSelectors).
379382
TTL(o.TTL).
380-
StorageLocation(o.StorageLocation).
383+
StorageLocation(storageLocation).
381384
VolumeSnapshotLocations(o.SnapshotLocations...).
382385
CSISnapshotTimeout(o.CSISnapshotTimeout).
383386
ItemOperationTimeout(o.ItemOperationTimeout).

‎cmd/non-admin/backup/describe.go‎

Lines changed: 80 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,12 @@ func NewDescribeCommand(f client.Factory, use string) *cobra.Command {
2929
Args: cobra.ExactArgs(1),
3030
RunE: func(cmd *cobra.Command, args []string) error {
3131
backupName := args[0]
32-
userNamespace := f.Namespace()
32+
33+
// Get the current namespace from kubectl context
34+
userNamespace, err := getCurrentNamespace()
35+
if err != nil {
36+
return fmt.Errorf("failed to determine current namespace: %w", err)
37+
}
3338

3439
// Setup scheme and client for NonAdminBackup resources
3540
scheme := runtime.NewScheme()
@@ -39,6 +44,9 @@ func NewDescribeCommand(f client.Factory, use string) *cobra.Command {
3944
if err := velerov1.AddToScheme(scheme); err != nil {
4045
return fmt.Errorf("failed to add Velero types to scheme: %w", err)
4146
}
47+
if err := corev1.AddToScheme(scheme); err != nil {
48+
return fmt.Errorf("failed to add core v1 types to scheme: %w", err)
49+
}
4250

4351
restConfig, err := f.ClientConfig()
4452
if err != nil {
@@ -128,6 +136,15 @@ func NonAdminDescribeBackup(cmd *cobra.Command, kbClient kbclient.Client, nab *n
128136
if nab.Status.VeleroBackup != nil && nab.Status.VeleroBackup.Name != "" {
129137
veleroBackupName := nab.Status.VeleroBackup.Name
130138

139+
// Try to get additional backup details, but don't block if they're not available
140+
fmt.Fprintf(cmd.OutOrStdout(), "\nFetching additional backup details...")
141+
142+
// Get backup results using NonAdminDownloadRequest (most important data)
143+
if results, err := downloadBackupData(ctx, kbClient, userNamespace, veleroBackupName, "BackupResults"); err == nil {
144+
fmt.Fprintf(cmd.OutOrStdout(), "\nBackup Results:\n")
145+
fmt.Fprintf(cmd.OutOrStdout(), "%s", indent(results, " "))
146+
}
147+
131148
// Get backup details using NonAdminDownloadRequest for BackupResourceList
132149
if resourceList, err := downloadBackupData(ctx, kbClient, userNamespace, veleroBackupName, "BackupResourceList"); err == nil {
133150
fmt.Fprintf(cmd.OutOrStdout(), "\nBackup Resource List:\n")
@@ -146,11 +163,7 @@ func NonAdminDescribeBackup(cmd *cobra.Command, kbClient kbclient.Client, nab *n
146163
fmt.Fprintf(cmd.OutOrStdout(), "%s", indent(itemOps, " "))
147164
}
148165

149-
// Get backup results using NonAdminDownloadRequest
150-
if results, err := downloadBackupData(ctx, kbClient, userNamespace, veleroBackupName, "BackupResults"); err == nil {
151-
fmt.Fprintf(cmd.OutOrStdout(), "\nBackup Results:\n")
152-
fmt.Fprintf(cmd.OutOrStdout(), "%s", indent(results, " "))
153-
}
166+
fmt.Fprintf(cmd.OutOrStdout(), "\nDone fetching additional details.")
154167
}
155168

156169
// Print NonAdminBackup Spec (excluding sensitive information)
@@ -159,39 +172,19 @@ func NonAdminDescribeBackup(cmd *cobra.Command, kbClient kbclient.Client, nab *n
159172
if err != nil {
160173
fmt.Fprintf(cmd.OutOrStdout(), "\nSpec: <error marshaling spec: %v>\n", err)
161174
} else {
162-
lines := strings.Split(string(specYaml), "\n")
163-
var filtered []string
164-
skip := false
165-
for i := 0; i < len(lines); i++ {
166-
line := lines[i]
167-
trimmed := strings.TrimSpace(line)
168-
if !skip && (strings.HasPrefix(trimmed, "includedNamespaces:") || strings.HasPrefix(trimmed, "includednamespaces:")) {
169-
skip = true
170-
continue
171-
}
172-
if skip {
173-
// Skip all list items or indented lines after the key
174-
if strings.HasPrefix(trimmed, "- ") || strings.HasPrefix(line, " ") || strings.HasPrefix(line, "\t") || trimmed == "" {
175-
continue
176-
} else {
177-
// Found a new top-level key, stop skipping
178-
skip = false
179-
}
180-
}
181-
if !skip {
182-
filtered = append(filtered, line)
183-
}
184-
}
185-
fmt.Fprintf(cmd.OutOrStdout(), "\nSpec:\n%s", indent(strings.Join(filtered, "\n"), " "))
175+
filteredSpec := filterIncludedNamespaces(string(specYaml))
176+
fmt.Fprintf(cmd.OutOrStdout(), "\nSpec:\n%s", indent(filteredSpec, " "))
186177
}
187178
}
188179

189-
// Print NonAdminBackup Status
180+
// Print NonAdminBackup Status (excluding sensitive information)
190181
statusYaml, err := yaml.Marshal(nab.Status)
191182
if err != nil {
192183
fmt.Fprintf(cmd.OutOrStdout(), "\nStatus: <error marshaling status: %v>\n", err)
193184
} else {
194-
fmt.Fprintf(cmd.OutOrStdout(), "\nStatus:\n%s", indent(string(statusYaml), " "))
185+
// Filter out includednamespaces from status output as well
186+
filteredStatus := filterIncludedNamespaces(string(statusYaml))
187+
fmt.Fprintf(cmd.OutOrStdout(), "\nStatus:\n%s", indent(filteredStatus, " "))
195188
}
196189

197190
// Print Events for NonAdminBackup
@@ -249,7 +242,7 @@ func downloadBackupData(ctx context.Context, kbClient kbclient.Client, userNames
249242
}()
250243

251244
// Wait for the download request to be processed
252-
timeout := time.After(30 * time.Second)
245+
timeout := time.After(10 * time.Second) // Reduced timeout since most failures are quick
253246
tick := time.Tick(1 * time.Second)
254247

255248
for {
@@ -265,16 +258,21 @@ func downloadBackupData(ctx context.Context, kbClient kbclient.Client, userNames
265258
return "", fmt.Errorf("failed to get NonAdminDownloadRequest: %w", err)
266259
}
267260

268-
switch updated.Status.Phase {
269-
case "Processed":
270-
if updated.Status.VeleroDownloadRequest.Status.DownloadURL != "" {
271-
// Download and return the content
272-
return downloadContent(updated.Status.VeleroDownloadRequest.Status.DownloadURL)
261+
// Check if the download request was processed successfully
262+
for _, condition := range updated.Status.Conditions {
263+
if condition.Type == "Processed" && condition.Status == "True" {
264+
if updated.Status.VeleroDownloadRequest.Status.DownloadURL != "" {
265+
// Download and return the content
266+
return downloadContent(updated.Status.VeleroDownloadRequest.Status.DownloadURL)
267+
}
268+
}
269+
}
270+
271+
// Check for failure conditions
272+
for _, condition := range updated.Status.Conditions {
273+
if condition.Status == "True" && condition.Reason == "Error" {
274+
return "", fmt.Errorf("NonAdminDownloadRequest failed for %s: %s - %s", dataType, condition.Type, condition.Message)
273275
}
274-
case "Failed":
275-
return "", fmt.Errorf("NonAdminDownloadRequest failed for %s: phase=%s", dataType, updated.Status.Phase)
276-
default:
277-
// Continue waiting
278276
}
279277
}
280278
}
@@ -313,6 +311,46 @@ func downloadContent(url string) (string, error) {
313311
return string(content), nil
314312
}
315313

314+
// Helper to filter out includednamespaces from YAML output
315+
func filterIncludedNamespaces(yamlContent string) string {
316+
lines := strings.Split(yamlContent, "\n")
317+
var filtered []string
318+
skip := false
319+
var skipIndentLevel int
320+
321+
for i := 0; i < len(lines); i++ {
322+
line := lines[i]
323+
trimmed := strings.TrimSpace(line)
324+
325+
// Calculate indentation level
326+
indentLevel := len(line) - len(strings.TrimLeft(line, " \t"))
327+
328+
// Check if this line starts the includednamespaces field
329+
if !skip && (trimmed == "includednamespaces:" || trimmed == "includedNamespaces:" ||
330+
strings.HasPrefix(trimmed, "includednamespaces: ") || strings.HasPrefix(trimmed, "includedNamespaces: ")) {
331+
skip = true
332+
skipIndentLevel = indentLevel
333+
continue
334+
}
335+
336+
if skip {
337+
// Stop skipping if we found a line at the same or lesser indentation level
338+
// and it's not an empty line and it's not a list item belonging to the skipped field
339+
if trimmed != "" && indentLevel <= skipIndentLevel && !strings.HasPrefix(trimmed, "- ") {
340+
skip = false
341+
// Process this line since we're no longer skipping
342+
filtered = append(filtered, line)
343+
}
344+
// If we're still skipping, don't add the line
345+
continue
346+
}
347+
348+
// Add the line if we're not skipping
349+
filtered = append(filtered, line)
350+
}
351+
return strings.Join(filtered, "\n")
352+
}
353+
316354
// Helper to indent YAML blocks
317355
func indent(s, prefix string) string {
318356
lines := strings.Split(s, "\n")

‎cmd/non-admin/backup/logs.go‎

Lines changed: 35 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,11 @@ func NewLogsCommand(f client.Factory, use string) *cobra.Command {
2727
ctx, cancel := context.WithTimeout(context.Background(), 120*time.Second)
2828
defer cancel()
2929

30-
userNamespace := f.Namespace()
30+
// Get the current namespace from kubectl context
31+
userNamespace, err := getCurrentNamespace()
32+
if err != nil {
33+
return fmt.Errorf("failed to determine current namespace: %w", err)
34+
}
3135
backupName := args[0]
3236

3337
scheme := runtime.NewScheme()
@@ -46,6 +50,15 @@ func NewLogsCommand(f client.Factory, use string) *cobra.Command {
4650
return fmt.Errorf("failed to create controller-runtime client: %w", err)
4751
}
4852

53+
// Verify the NonAdminBackup exists before creating download request
54+
var nab nacv1alpha1.NonAdminBackup
55+
if err := kbClient.Get(ctx, kbclient.ObjectKey{
56+
Namespace: userNamespace,
57+
Name: backupName,
58+
}, &nab); err != nil {
59+
return fmt.Errorf("failed to get NonAdminBackup %q: %w", backupName, err)
60+
}
61+
4962
req := &nacv1alpha1.NonAdminDownloadRequest{
5063
ObjectMeta: metav1.ObjectMeta{
5164
GenerateName: backupName + "-logs-",
@@ -54,7 +67,7 @@ func NewLogsCommand(f client.Factory, use string) *cobra.Command {
5467
Spec: nacv1alpha1.NonAdminDownloadRequestSpec{
5568
Target: velerov1.DownloadTarget{
5669
Kind: "BackupLog",
57-
Name: backupName,
70+
Name: backupName, // Use NonAdminBackup name, controller will resolve to Velero backup
5871
},
5972
},
6073
}
@@ -70,14 +83,17 @@ func NewLogsCommand(f client.Factory, use string) *cobra.Command {
7083
}()
7184

7285
var signedURL string
73-
timeout := time.After(60 * time.Second)
74-
tick := time.Tick(1 * time.Second)
86+
timeout := time.After(120 * time.Second) // Increased timeout to 2 minutes
87+
tick := time.Tick(2 * time.Second) // Check every 2 seconds instead of 1
88+
89+
fmt.Fprintf(cmd.OutOrStdout(), "Waiting for backup logs to be processed...")
7590
Loop:
7691
for {
7792
select {
7893
case <-timeout:
7994
return fmt.Errorf("timed out waiting for NonAdminDownloadRequest to be processed")
8095
case <-tick:
96+
fmt.Fprintf(cmd.OutOrStdout(), ".")
8197
var updated nacv1alpha1.NonAdminDownloadRequest
8298
if err := kbClient.Get(ctx, kbclient.ObjectKey{
8399
Namespace: req.Namespace,
@@ -86,15 +102,22 @@ func NewLogsCommand(f client.Factory, use string) *cobra.Command {
86102
return fmt.Errorf("failed to get NonAdminDownloadRequest: %w", err)
87103
}
88104

89-
switch updated.Status.Phase {
90-
case "Processed":
91-
if updated.Status.VeleroDownloadRequest.Status.DownloadURL != "" {
92-
signedURL = updated.Status.VeleroDownloadRequest.Status.DownloadURL
93-
break Loop
105+
// Check if the download request was processed successfully
106+
for _, condition := range updated.Status.Conditions {
107+
if condition.Type == "Processed" && condition.Status == "True" {
108+
if updated.Status.VeleroDownloadRequest.Status.DownloadURL != "" {
109+
signedURL = updated.Status.VeleroDownloadRequest.Status.DownloadURL
110+
fmt.Fprintf(cmd.OutOrStdout(), "\nDownload URL received, fetching logs...\n")
111+
break Loop
112+
}
113+
}
114+
}
115+
116+
// Check for failure conditions
117+
for _, condition := range updated.Status.Conditions {
118+
if condition.Status == "True" && condition.Reason == "Error" {
119+
return fmt.Errorf("NonAdminDownloadRequest failed: %s - %s", condition.Type, condition.Message)
94120
}
95-
case "Failed":
96-
return fmt.Errorf("NonAdminDownloadRequest failed: phase=%s", updated.Status.Phase)
97-
default:
98121
}
99122
}
100123
}

0 commit comments

Comments
 (0)