Skip to content

Commit

Permalink
chore: improve error logs (argoproj#10592) (argoproj#19743)
Browse files Browse the repository at this point in the history 
Signed-off-by: KangManJoo <eogns47@konkuk.ac.kr>
  • Loading branch information
@eogns47
eogns47 authored Sep 11, 2024
1 parent a7bc623 commit b098f21
Show file tree
Hide file tree
Showing 3 changed files with 102 additions and 61 deletions.
68 changes: 38 additions & 30 deletions util/helm/client.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ func fileExist(filePath string) (bool, error) {
if os.IsNotExist(err) {
return false, nil
} else {
return false, err
return false, fmt.Errorf("error checking file existence for %s: %w", filePath, err)
}
}
return true, nil
Expand All @@ -122,21 +122,27 @@ func fileExist(filePath string) (bool, error) {
func (c *nativeHelmChart) CleanChartCache(chart string, version string, project string) error {
cachePath, err := c.getCachedChartPath(chart, version, project)
if err != nil {
return err
return fmt.Errorf("error getting cached chart path: %w", err)
}
return os.RemoveAll(cachePath)
if err := os.RemoveAll(cachePath); err != nil {
return fmt.Errorf("error removing chart cache at %s: %w", cachePath, err)
}
return nil
}

func untarChart(tempDir string, cachedChartPath string, manifestMaxExtractedSize int64, disableManifestMaxExtractedSize bool) error {
if disableManifestMaxExtractedSize {
cmd := exec.Command("tar", "-zxvf", cachedChartPath)
cmd.Dir = tempDir
_, err := executil.Run(cmd)
return err
if err != nil {
return fmt.Errorf("error executing tar command: %w", err)
}
return nil
}
reader, err := os.Open(cachedChartPath)
if err != nil {
return err
return fmt.Errorf("error opening cached chart path %s: %w", cachedChartPath, err)
}
return files.Untgz(tempDir, reader, manifestMaxExtractedSize, false)
}
Expand All @@ -145,19 +151,19 @@ func (c *nativeHelmChart) ExtractChart(chart string, version string, project str
// always use Helm V3 since we don't have chart content to determine correct Helm version
helmCmd, err := NewCmdWithVersion("", c.enableOci, c.proxy, c.noProxy)
if err != nil {
return "", nil, err
return "", nil, fmt.Errorf("error creating Helm command: %w", err)
}
defer helmCmd.Close()

// throw away temp directory that stores extracted chart and should be deleted as soon as no longer needed by returned closer
tempDir, err := files.CreateTempDir(os.TempDir())
if err != nil {
return "", nil, err
return "", nil, fmt.Errorf("error creating temporary directory: %w", err)
}

cachedChartPath, err := c.getCachedChartPath(chart, version, project)
if err != nil {
return "", nil, err
return "", nil, fmt.Errorf("error getting cached chart path: %w", err)
}

c.repoLock.Lock(cachedChartPath)
Expand All @@ -166,22 +172,22 @@ func (c *nativeHelmChart) ExtractChart(chart string, version string, project str
// check if chart tar is already downloaded
exists, err := fileExist(cachedChartPath)
if err != nil {
return "", nil, err
return "", nil, fmt.Errorf("error checking existence of cached chart path: %w", err)
}

if !exists {
// create empty temp directory to extract chart from the registry
tempDest, err := files.CreateTempDir(os.TempDir())
if err != nil {
return "", nil, err
return "", nil, fmt.Errorf("error creating temporary destination directory: %w", err)
}
defer func() { _ = os.RemoveAll(tempDest) }()

if c.enableOci {
if c.creds.Password != "" && c.creds.Username != "" {
_, err = helmCmd.RegistryLogin(c.repoURL, c.creds)
if err != nil {
return "", nil, err
return "", nil, fmt.Errorf("error logging into OCI registry: %w", err)
}

defer func() {
Expand All @@ -192,34 +198,36 @@ func (c *nativeHelmChart) ExtractChart(chart string, version string, project str
// 'helm pull' ensures that chart is downloaded into temp directory
_, err = helmCmd.PullOCI(c.repoURL, chart, version, tempDest, c.creds)
if err != nil {
return "", nil, err
return "", nil, fmt.Errorf("error pulling OCI chart: %w", err)
}
} else {
_, err = helmCmd.Fetch(c.repoURL, chart, version, tempDest, c.creds, passCredentials)
if err != nil {
return "", nil, err
return "", nil, fmt.Errorf("error fetching chart: %w", err)
}
}

// 'helm pull/fetch' file downloads chart into the tgz file and we move that to where we want it
infos, err := os.ReadDir(tempDest)
if err != nil {
return "", nil, err
return "", nil, fmt.Errorf("error reading directory %s: %w", tempDest, err)
}
if len(infos) != 1 {
return "", nil, fmt.Errorf("expected 1 file, found %v", len(infos))
}

err = os.Rename(filepath.Join(tempDest, infos[0].Name()), cachedChartPath)
chartFilePath := filepath.Join(tempDest, infos[0].Name())

err = os.Rename(chartFilePath, cachedChartPath)
if err != nil {
return "", nil, err
return "", nil, fmt.Errorf("error renaming file from %s to %s: %w", chartFilePath, cachedChartPath, err)
}
}

err = untarChart(tempDir, cachedChartPath, manifestMaxExtractedSize, disableManifestMaxExtractedSize)
if err != nil {
_ = os.RemoveAll(tempDir)
return "", nil, err
return "", nil, fmt.Errorf("error untarring chart: %w", err)
}
return path.Join(tempDir, normalizeChartName(chart)), argoio.NewCloser(func() error {
return os.RemoveAll(tempDir)
Expand All @@ -242,7 +250,7 @@ func (c *nativeHelmChart) GetIndex(noCache bool, maxIndexSize int64) (*Index, er
var err error
data, err = c.loadRepoIndex(maxIndexSize)
if err != nil {
return nil, err
return nil, fmt.Errorf("error loading repo index: %w", err)
}
log.WithFields(log.Fields{"seconds": time.Since(start).Seconds()}).Info("took to get index")

Expand All @@ -256,7 +264,7 @@ func (c *nativeHelmChart) GetIndex(noCache bool, maxIndexSize int64) (*Index, er
index := &Index{}
err := yaml.NewDecoder(bytes.NewBuffer(data)).Decode(index)
if err != nil {
return nil, err
return nil, fmt.Errorf("error decoding index: %w", err)
}

return index, nil
Expand All @@ -267,13 +275,13 @@ func (c *nativeHelmChart) TestHelmOCI() (bool, error) {

tmpDir, err := os.MkdirTemp("", "helm")
if err != nil {
return false, err
return false, fmt.Errorf("error creating temporary directory: %w", err)
}
defer func() { _ = os.RemoveAll(tmpDir) }()

helmCmd, err := NewCmdWithVersion(tmpDir, c.enableOci, c.proxy, c.noProxy)
if err != nil {
return false, err
return false, fmt.Errorf("error creating Helm command: %w", err)
}
defer helmCmd.Close()

Expand All @@ -282,7 +290,7 @@ func (c *nativeHelmChart) TestHelmOCI() (bool, error) {
if c.creds.Username != "" && c.creds.Password != "" {
_, err = helmCmd.RegistryLogin(c.repoURL, c.creds)
if err != nil {
return false, err
return false, fmt.Errorf("error logging into OCI registry: %w", err)
}
defer func() {
_, _ = helmCmd.RegistryLogout(c.repoURL, c.creds)
Expand All @@ -296,12 +304,12 @@ func (c *nativeHelmChart) TestHelmOCI() (bool, error) {
func (c *nativeHelmChart) loadRepoIndex(maxIndexSize int64) ([]byte, error) {
indexURL, err := getIndexURL(c.repoURL)
if err != nil {
return nil, err
return nil, fmt.Errorf("error getting index URL: %w", err)
}

req, err := http.NewRequest(http.MethodGet, indexURL, nil)
if err != nil {
return nil, err
return nil, fmt.Errorf("error creating HTTP request: %w", err)
}
if c.creds.Username != "" || c.creds.Password != "" {
// only basic supported
Expand All @@ -310,7 +318,7 @@ func (c *nativeHelmChart) loadRepoIndex(maxIndexSize int64) ([]byte, error) {

tlsConf, err := newTLSConfig(c.creds)
if err != nil {
return nil, err
return nil, fmt.Errorf("error creating TLS config: %w", err)
}

tr := &http.Transport{
Expand All @@ -321,7 +329,7 @@ func (c *nativeHelmChart) loadRepoIndex(maxIndexSize int64) ([]byte, error) {
client := http.Client{Transport: tr}
resp, err := client.Do(req)
if err != nil {
return nil, err
return nil, fmt.Errorf("error making HTTP request: %w", err)
}
defer func() { _ = resp.Body.Close() }()

Expand All @@ -337,7 +345,7 @@ func newTLSConfig(creds Creds) (*tls.Config, error) {
if creds.CAPath != "" {
caData, err := os.ReadFile(creds.CAPath)
if err != nil {
return nil, err
return nil, fmt.Errorf("error reading CA file %s: %w", creds.CAPath, err)
}
caCertPool := x509.NewCertPool()
caCertPool.AppendCertsFromPEM(caData)
Expand All @@ -348,7 +356,7 @@ func newTLSConfig(creds Creds) (*tls.Config, error) {
if len(creds.CertData) > 0 && len(creds.KeyData) > 0 {
cert, err := tls.X509KeyPair(creds.CertData, creds.KeyData)
if err != nil {
return nil, err
return nil, fmt.Errorf("error creating X509 key pair: %w", err)
}
tlsConfig.Certificates = []tls.Certificate{cert}
}
Expand All @@ -373,7 +381,7 @@ func normalizeChartName(chart string) string {
func (c *nativeHelmChart) getCachedChartPath(chart string, version string, project string) (string, error) {
keyData, err := json.Marshal(map[string]string{"url": c.repoURL, "chart": chart, "version": version, "project": project})
if err != nil {
return "", err
return "", fmt.Errorf("error marshaling cache key data: %w", err)
}
return c.chartCachePaths.GetPath(string(keyData))
}
Expand All @@ -392,7 +400,7 @@ func getIndexURL(rawURL string) (string, error) {
indexFile := "index.yaml"
repoURL, err := url.Parse(rawURL)
if err != nil {
return "", err
return "", fmt.Errorf("error parsing repository URL: %w", err)
}
repoURL.Path = path.Join(repoURL.Path, indexFile)
repoURL.RawPath = path.Join(repoURL.RawPath, indexFile)
Expand Down
Loading

0 comments on commit b098f21

Please sign in to comment.