Give more information for errors of failed chart repo index #135
Conversation
ghost
marked this pull request as ready for review
pkg/chart/repo/handlers.go
Outdated
| @@ -22,6 +22,10 @@ func ResolveChartVersionFunc(c *Catalog) ChartVersionResolver { | |||
|
|
|||
| if _, err := repo.RefreshIndex(); err != nil { | |||
| glog.Warningf("failed to refresh repo[%s] index: %s", chartspec.RepoURL, err) | |||
| switch err.(type) { | |||
| case errors.NoCachedChartRepoIndexError: | |||
| return nil, err | |||
There was a problem hiding this comment.
I suggest adding default: statement (even an empty one if a fall through is meant here) so the alternative action is explicitly visible.
| @@ -61,6 +61,13 @@ func (r *Repo) RefreshIndex() (*repo.IndexFile, error) { | |||
|
|
|||
| data, err := r.fetcher(indexURL) | |||
| if err != nil { | |||
| _, cacheErr := r.cache.Fetch("index.yaml") | |||
| if cacheErr != nil { | |||
There was a problem hiding this comment.
Do we care about cacheErr contents? It seems it's lost after a non-nil check
There was a problem hiding this comment.
The error that we are printing to logs is the response code from the server when trying to fetch the chart (for example: "unsuccessful response code: 500 Internal Server Error (500)"). cacheErr will hold the error for trying to fetch from cache (for example: "open /tmp/chart-cache/https-charts.prod.booking.com-charts/index.yaml: no such file or directory")
I can add cacheErr if you think it should be there as well
There was a problem hiding this comment.
I see your point. It seems to be a composite error: we got err and cacheErr, both worth reporting. I might suggest using shippererrors.MultiError in that case so we don't miss the actual err reason contained in err.
ghost
changed the title
|
It looks like we have conflicts from when #143 got merged. @hbarkal can you rebase? |
ghost
force-pushed
the
hbarkal/handle-failed-chart-repo-index
branch
4 times, most recently
from
d8aaa5b to
8d8e1fa
Compare
|
@hbarkal we have conflicts again after recent refactorings on repo index fetching. Can you rebase again? |
ghost
force-pushed
the
hbarkal/handle-failed-chart-repo-index
branch
from
8d8e1fa to
1a67d9c
Compare
Add an error for no chart repo to give more information to the user This error will be used when we failed to fetch chart repo, and there's no previous index in cache for fallback
ghost
force-pushed
the
hbarkal/handle-failed-chart-repo-index
branch
from
d2b7ea2 to
d3f668d
Compare
Add an error for no chart repo to give more information to the user
This error will be used when we failed to fetch chart repo, and there's no previous index in cache for fallback