Skip to content

Commit 9970fb1

Browse files
authored
feat: [sc-114709] [troubleshoot] gracefully handle inability to load specs if multiple specs provided (#1686)
gracefully handle unable to load URL spec
1 parent 0213fbb commit 9970fb1

File tree

2 files changed

+65
-23
lines changed

2 files changed

+65
-23
lines changed

internal/specs/specs.go

Lines changed: 22 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"strings"
1212
"time"
1313

14+
"github.com/fatih/color"
1415
"github.com/pkg/errors"
1516
"github.com/replicatedhq/troubleshoot/internal/util"
1617
"github.com/replicatedhq/troubleshoot/pkg/constants"
@@ -78,7 +79,7 @@ func LoadFromCLIArgs(ctx context.Context, client kubernetes.Interface, args []st
7879
ctx = context.Background()
7980
}
8081

81-
var kindsFromURL *loader.TroubleshootKinds
82+
allURLSpecs := loader.NewTroubleshootKinds()
8283
rawSpecs := []string{}
8384

8485
for _, v := range args {
@@ -176,34 +177,33 @@ func LoadFromCLIArgs(ctx context.Context, client kubernetes.Interface, args []st
176177
return nil, types.NewExitCodeError(constants.EXIT_CODE_SPEC_ISSUES, err)
177178
}
178179

179-
var specFromURL string
180+
var rawURLSpec string
181+
headers := map[string]string{}
182+
180183
if parsedURL.Host == "kots.io" {
181-
// To download specs from kots.io, we need to set the User-Agent header
182-
specFromURL, err = downloadFromHttpURL(ctx, v, map[string]string{
183-
"User-Agent": "Replicated_Troubleshoot/v1beta1",
184-
})
185-
if err != nil {
186-
return nil, err
187-
}
188-
} else {
189-
specFromURL, err = downloadFromHttpURL(ctx, v, nil)
190-
if err != nil {
191-
return nil, err
192-
}
184+
headers["User-Agent"] = "Replicated_Troubleshoot/v1beta1"
185+
}
186+
rawURLSpec, err = downloadFromHttpURL(ctx, v, headers)
187+
if err != nil {
188+
fmt.Println(color.YellowString("failed to download spec from URI %q: %v\n", v, err))
189+
continue
193190
}
194191

195192
// load URL spec first to remove URI key from the spec
196-
kindsFromURL, err = loader.LoadSpecs(ctx, loader.LoadOptions{
197-
RawSpec: specFromURL,
193+
urlSpec, err := loader.LoadSpecs(ctx, loader.LoadOptions{
194+
RawSpec: rawURLSpec,
198195
})
199196
if err != nil {
200-
return nil, err
197+
fmt.Println(color.YellowString("failed to load spec from URI %q: %v\n", v, err))
198+
continue
201199
}
202200
// remove URI key from the spec if any
203-
for i := range kindsFromURL.SupportBundlesV1Beta2 {
204-
kindsFromURL.SupportBundlesV1Beta2[i].Spec.Uri = ""
201+
for i := range urlSpec.SupportBundlesV1Beta2 {
202+
urlSpec.SupportBundlesV1Beta2[i].Spec.Uri = ""
205203
}
206204

205+
allURLSpecs.Add(urlSpec)
206+
207207
}
208208
}
209209
}
@@ -214,8 +214,8 @@ func LoadFromCLIArgs(ctx context.Context, client kubernetes.Interface, args []st
214214
if err != nil {
215215
return nil, err
216216
}
217-
if kindsFromURL != nil {
218-
kinds.Add(kindsFromURL)
217+
if allURLSpecs.Len() > 0 {
218+
kinds.Add(allURLSpecs)
219219
}
220220

221221
if vp.GetBool("load-cluster-specs") {
@@ -224,8 +224,7 @@ func LoadFromCLIArgs(ctx context.Context, client kubernetes.Interface, args []st
224224
if kinds.IsEmpty() {
225225
return nil, types.NewExitCodeError(constants.EXIT_CODE_SPEC_ISSUES, err)
226226
}
227-
// TODO: Consider colour coding and graceful failures when loading specs
228-
fmt.Printf("failed to load specs from the cluster: %v\n", err)
227+
fmt.Println(color.YellowString("failed to load specs from cluster: %v\n", err))
229228
} else {
230229
kinds.Add(clusterKinds)
231230
}

internal/specs/specs_test.go

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,13 @@
11
package specs
22

33
import (
4+
"bytes"
45
"context"
56
"fmt"
7+
"io"
68
"net/http"
79
"net/http/httptest"
10+
"os"
811
"reflect"
912
"testing"
1013

@@ -225,6 +228,46 @@ spec:
225228
require.Len(t, specs.HostCollectorsV1Beta2, 1)
226229
}
227230

231+
func TestLoadFromMultipleURIs(t *testing.T) {
232+
server1 := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
233+
w.Write([]byte(`apiVersion: troubleshoot.sh/v1beta2
234+
kind: HostCollector
235+
metadata:
236+
name: cpu
237+
spec:
238+
collectors:
239+
- cpu: {}
240+
`))
241+
}))
242+
defer server1.Close()
243+
244+
server2 := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
245+
http.Error(w, "Internal Server Error", http.StatusInternalServerError)
246+
}))
247+
// immediately close the server to force the error
248+
server2.Close()
249+
250+
// Capture stdout
251+
oldStdout := os.Stdout
252+
r, w, _ := os.Pipe()
253+
os.Stdout = w
254+
255+
client := testclient.NewSimpleClientset()
256+
specs, err := LoadFromCLIArgs(context.Background(), client, []string{server1.URL, server2.URL}, viper.New())
257+
258+
// Restore stdout
259+
w.Close()
260+
os.Stdout = oldStdout
261+
262+
var buf bytes.Buffer
263+
io.Copy(&buf, r)
264+
output := buf.String()
265+
266+
require.NoError(t, err)
267+
require.Len(t, specs.HostCollectorsV1Beta2, 1)
268+
assert.Contains(t, output, "failed to download spec from URI")
269+
}
270+
228271
func TestLoadAdditionalSpecFromURIs(t *testing.T) {
229272
m := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
230273
w.Write([]byte(`apiVersion: troubleshoot.sh/v1beta2

0 commit comments

Comments
 (0)