Skip to content

fix(versions): log release versions when config is missing #745

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

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,7 @@ jobs:
HHFAB_VLAB_COLLECT: true
if: ${{ env.run_release_test == 'true' }}
run: |
bin/hhfab versions
bin/hhfab init -v --dev -m ${{ matrix.fabricmode }} --include-onie=${{ matrix.includeonie }} --gateway=${{ matrix.gateway }}
bin/hhfab vlab gen -v
export HHFAB_JOIN_TOKEN=$(openssl rand -base64 24)
Expand All @@ -337,6 +338,7 @@ jobs:
HHFAB_VLAB_COLLECT: true
if: ${{ env.run_release_test != 'true' }}
run: |
bin/hhfab versions
bin/hhfab init -v --dev -m ${{ matrix.fabricmode }} --include-onie=${{ matrix.includeonie }} --gateway=${{ matrix.gateway }}
bin/hhfab vlab gen -v
bin/hhfab diagram -f mermaid
Expand Down Expand Up @@ -435,6 +437,7 @@ jobs:
curl -fsSL https://i.hhdev.io/hhfab | USE_SUDO=false INSTALL_DIR=./old VERSION="${{ matrix.fromversion }}" bash

old/hhfab init -v --dev -m ${{ matrix.fabricmode }} --include-onie=true
old/hhfab versions
old/hhfab vlab gen -v
old/hhfab vlab up -v --ready setup-vpcs --ready test-connectivity --ready exit ${{ matrix.fromflags }}

Expand All @@ -454,6 +457,7 @@ jobs:
HHFAB_REG_REPO: 127.0.0.1:30000
HHFAB_VLAB_COLLECT: true
run: |
bin/hhfab versions
bin/hhfab vlab up -v --ready inspect --ready setup-vpcs --ready test-connectivity --ready exit --upgrade

- name: Upload show-tech artifacts
Expand All @@ -468,6 +472,7 @@ jobs:
HHFAB_REG_REPO: 127.0.0.1:30000
HHFAB_VLAB_COLLECT: true
run: |
bin/hhfab versions
bin/hhfab vlab up -v --ready wait --ready exit -m=manual

- name: Upload show-tech artifacts
Expand Down Expand Up @@ -574,6 +579,7 @@ jobs:
if: ${{ env.run_release_test == 'true' }}
run: |
source "./lab-ci/envs/$KUBE_NODE/source.sh"
bin/hhfab versions
bin/hhfab init -v --dev --include-onie=${{ matrix.includeonie }} -w "./lab-ci/envs/$KUBE_NODE/wiring.yaml"

# TODO: make controls restricted again when we figure out how to get NTP upstream working for isolated VMs
Expand All @@ -594,6 +600,7 @@ jobs:
if: ${{ env.run_release_test != 'true' }}
run: |
source "./lab-ci/envs/$KUBE_NODE/source.sh"
bin/hhfab versions
bin/hhfab init -v --dev --include-onie=${{ matrix.includeonie }} -w "./lab-ci/envs/$KUBE_NODE/wiring.yaml"
bin/hhfab diagram -f mermaid

Expand Down
15 changes: 10 additions & 5 deletions cmd/hhfab/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -629,12 +629,17 @@ func Run(ctx context.Context) error {
},
},
{
Name: "versions",
Usage: "print versions of all components",
Flags: flatten(defaultFlags, hModeFlags),
Name: "versions",
Usage: "print versions of all components",
Flags: flatten(defaultFlags, hModeFlags, []cli.Flag{
&cli.BoolFlag{
Name: "live",
Usage: "load versions from running API instead of the config file (fab.yaml)",
},
}),
Before: before(false),
Action: func(_ *cli.Context) error {
if err := hhfab.Versions(ctx, workDir, cacheDir, hhfab.HydrateMode(hydrateMode)); err != nil {
Action: func(c *cli.Context) error {
if err := hhfab.Versions(ctx, workDir, cacheDir, hhfab.HydrateMode(hydrateMode), c.Bool("live")); err != nil {
return fmt.Errorf("printing versions: %w", err)
}

Expand Down
18 changes: 0 additions & 18 deletions pkg/hhfab/cmdconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -267,24 +267,6 @@ func Validate(ctx context.Context, workDir, cacheDir string, hMode HydrateMode)
return nil
}

func Versions(ctx context.Context, workDir, cacheDir string, hMode HydrateMode) error {
cfg, err := load(ctx, workDir, cacheDir, true, hMode, "")
if err != nil {
return err
}

slog.Info("Printing versions of all components")

data, err := kyaml.Marshal(cfg.Fab.Status.Versions)
if err != nil {
return fmt.Errorf("marshalling versions: %w", err)
}

fmt.Println(string(data))

return nil
}

func load(ctx context.Context, workDir, cacheDir string, wiringAndHydration bool, mode HydrateMode, setJoinToken string) (*Config, error) {
if err := checkWorkCacheDir(workDir, cacheDir); err != nil {
return nil, err
Expand Down
171 changes: 171 additions & 0 deletions pkg/hhfab/cmdversions.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,171 @@
// Copyright 2024 Hedgehog
// SPDX-License-Identifier: Apache-2.0

package hhfab

import (
"context"
"errors"
"fmt"
"log/slog"
"os"
"path/filepath"

"go.githedgehog.com/fabric/pkg/util/kubeutil"
fabapi "go.githedgehog.com/fabricator/api/fabricator/v1beta1"
"go.githedgehog.com/fabricator/pkg/fab"
kclient "sigs.k8s.io/controller-runtime/pkg/client"
kyaml "sigs.k8s.io/yaml"
)

func Versions(ctx context.Context, workDir, cacheDir string, hMode HydrateMode, live bool) error {
if live {
cfg := &Config{
WorkDir: workDir,
CacheDir: cacheDir,
}

return getVersionsFromCluster(ctx, cfg)
}

configPath := filepath.Join(workDir, FabConfigFile)
if _, err := os.Stat(configPath); err != nil && errors.Is(err, os.ErrNotExist) {
slog.Info("No configuration found", "file", FabConfigFile, "action", "Showing release versions")
freshFab := fabapi.Fabricator{}
if err := freshFab.CalculateVersions(fab.Versions); err != nil {
return fmt.Errorf("calculating default versions: %w", err)
}
data, err := kyaml.Marshal(freshFab.Status.Versions)
if err != nil {
return fmt.Errorf("marshalling versions: %w", err)
}
fmt.Println(string(data))

return nil
}

cfg, err := load(ctx, workDir, cacheDir, true, hMode, "")
if err != nil {
return err
}

freshFab := fabapi.Fabricator{}
if err := freshFab.CalculateVersions(fab.Versions); err != nil {
return fmt.Errorf("calculating default versions: %w", err)
}

versionsData, err := formatVersions(freshFab.Status.Versions, cfg.Fab.Status.Versions)
if err != nil {
return fmt.Errorf("formatting versions: %w", err)
}

fmt.Println(versionsData)

return nil
Comment on lines +52 to +64
Copy link
Preview

Copilot AI Jul 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] There are multiple duplicated blocks that call CalculateVersions and print results; consider extracting common code into a helper to reduce duplication.

Suggested change
freshFab := fabapi.Fabricator{}
if err := freshFab.CalculateVersions(fab.Versions); err != nil {
return fmt.Errorf("calculating default versions: %w", err)
}
versionsData, err := formatVersions(freshFab.Status.Versions, cfg.Fab.Status.Versions)
if err != nil {
return fmt.Errorf("formatting versions: %w", err)
}
fmt.Println(versionsData)
return nil
return processVersions(fab.Versions, cfg.Fab.Status.Versions, false)

Copilot uses AI. Check for mistakes.

}

func getVersionsFromCluster(ctx context.Context, c *Config) error {
kubeconfig := filepath.Join(c.WorkDir, VLABDir, VLABKubeConfig)
kube, err := kubeutil.NewClient(ctx, kubeconfig, fabapi.SchemeBuilder)
if err != nil {
return fmt.Errorf("creating kube client: %w", err)
}

fabObj := &fabapi.Fabricator{}
if err := kube.Get(ctx, kclient.ObjectKey{Name: "default", Namespace: "fab"}, fabObj); err != nil {
return fmt.Errorf("getting fabricator object: %w", err)
}

slog.Info("Printing versions from live cluster")

freshFab := fabapi.Fabricator{}
if err := freshFab.CalculateVersions(fab.Versions); err != nil {
return fmt.Errorf("calculating default versions: %w", err)
}

versionsData, err := formatVersions(freshFab.Status.Versions, fabObj.Status.Versions)
if err != nil {
return fmt.Errorf("formatting versions: %w", err)
}

fmt.Println(versionsData)

return nil
}

func formatVersions(releaseVersions, overriddenVersions fabapi.Versions) (string, error) {
releaseMap, err := convertToMap(releaseVersions)
if err != nil {
return "", fmt.Errorf("converting release versions to map: %w", err)
}

overriddenMap, err := convertToMap(overriddenVersions)
if err != nil {
slog.Warn("Failed to convert overridden versions", "error", err)
data, _ := kyaml.Marshal(releaseVersions)
Copy link
Preview

Copilot AI Jul 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoid ignoring the error from kyaml.Marshal here; handle or log it so that serialization failures in the fallback path are not silently dropped.

Suggested change
data, _ := kyaml.Marshal(releaseVersions)
data, marshalErr := kyaml.Marshal(releaseVersions)
if marshalErr != nil {
slog.Warn("Failed to marshal release versions", "error", marshalErr)
return "", fmt.Errorf("marshalling release versions: %w", marshalErr)
}

Copilot uses AI. Check for mistakes.


return string(data), nil
}

slog.Info("Printing versions of all components (release → override)")

result := compareVersionMaps(releaseMap, overriddenMap)

resultData, err := kyaml.Marshal(result)
if err != nil {
return "", fmt.Errorf("marshalling result: %w", err)
}

return string(resultData), nil
}

func convertToMap(v interface{}) (map[string]interface{}, error) {
data, err := kyaml.Marshal(v)
if err != nil {
return nil, fmt.Errorf("marshalling: %w", err)
}

var result map[string]interface{}
if err := kyaml.Unmarshal(data, &result); err != nil {
return nil, fmt.Errorf("unmarshalling: %w", err)
}

return result, nil
}

func compareVersionMaps(releases, overridden map[string]interface{}) map[string]interface{} {
result := make(map[string]interface{})

for key, releaseValue := range releases {
releaseMap, isMap := releaseValue.(map[string]interface{})
if isMap {
overriddenMap, hasOverridden := overridden[key].(map[string]interface{})
if hasOverridden {
result[key] = compareVersionMaps(releaseMap, overriddenMap)
} else {
result[key] = releaseMap
}
} else {
releaseStr, isString := releaseValue.(string)
if !isString {
result[key] = releaseValue

continue
}

overriddenValue, hasOverridden := overridden[key]
if hasOverridden {
overriddenStr, isString := overriddenValue.(string)
if isString && overriddenStr != "" && overriddenStr != releaseStr {
result[key] = fmt.Sprintf("%s → %s", releaseStr, overriddenStr)
} else {
result[key] = releaseValue
}
} else {
result[key] = releaseValue
}
}
}

return result
}
Loading