Skip to content
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

feat: add hold file for chart upgrade #1135

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft
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
2 changes: 1 addition & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,4 @@ mc
/arkade-*
/faas-cli*
test.out
docker-compose.yaml
docker-compose.yaml*
43 changes: 41 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -325,8 +325,8 @@ The directory that contains the Helm chart should be a Git repository. If the fl

There are two commands built into arkade designed for software vendors and open source maintainers.

* `arkade helm chart upgrade` - run this command to scan for container images and update them automatically by querying a remote registry.
* `arkade helm chart verify` - after changing the contents of a values.yaml or docker-compose.yaml file, this command will check each image exists on a remote registry
* `arkade chart upgrade` - run this command to scan for container images and update them automatically by querying a remote registry.
* `arkade chart verify` - after changing the contents of a values.yaml or docker-compose.yaml file, this command will check each image exists on a remote registry

Whilst end-users may use a GitOps-style tool to deploy charts and update their versions, maintainers need to make conscious decisions about when and which images to change within a Helm chart or compose file.

Expand Down Expand Up @@ -376,6 +376,45 @@ arkade chart upgrade -f \
--write
```

### Holding image versions within a Helm chart

With the command `arkade chart upgrade` you can add a `.hold` associated with a `values.yaml` file, e.g. `values.yaml.hold` to exclude the specified images from the version update.

Original YAML file:

```yaml
stan:
# Image used for nats deployment when using async with NATS-Streaming.
image: nats-streaming:0.24.6

db:
image: postgres:16
```

Associated hold file:

```
db.image
```

```bash
arkade chart upgrade -f \
~/go/src/github.com/openfaas/faas-netes/chart/openfaas/values.yaml \
--verbose

2023/01/03 10:12:47 Verifying images in: /home/alex/go/src/github.com/openfaas/faas-netes/chart/openfaas/values.yaml
2023/01/03 10:12:47 Found 18 images
2023/01/03 10:12:47 Found 1 image to hold/ignore in values.yaml.hold
2023/01/03 10:12:47 Processing 17 images
2023/01/03 10:12:48 [natsio/prometheus-nats-exporter] 0.8.0 => 0.10.1
2023/01/03 10:12:50 [nats-streaming] 0.24.6 => 0.25.2
2023/01/03 10:12:52 [prom/prometheus] v2.38.0 => 2.41.0
2023/01/03 10:12:54 [prom/alertmanager] v0.24.0 => 0.25.0
2023/01/03 10:12:54 [nats] 2.9.2 => 2.9.10
```

Within the upgrade activity `postgres:16` is no longer included.

Supported:

* `image:` - at the top level
Expand Down
77 changes: 70 additions & 7 deletions cmd/chart/upgrade.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package chart

import (
"bufio"
"errors"
"fmt"
"log"
Expand All @@ -24,6 +25,8 @@ type tagAttributes struct {
original string
}

const holdFileExt = "hold"

func (c *tagAttributes) attributesMatch(n tagAttributes) bool {
return c.hasMajor == n.hasMajor &&
c.hasMinor == n.hasMinor &&
Expand Down Expand Up @@ -90,15 +93,28 @@ Otherwise, it returns a non-zero exit code and the updated values.yaml file.`,
return err
}

filtered := helm.FilterImagesUptoDepth(values, depth)
filtered := helm.FilterImagesUptoDepth(values, depth, 0, "")
if len(filtered) == 0 {
return fmt.Errorf("no images found in %s", file)
}

if verbose {
if len(filtered) > 0 {
log.Printf("Found %d images\n", len(filtered))
}
log.Printf("Found %d image%s\n", len(filtered), pluralise(len(filtered)))
}

holdfile := fmt.Sprintf("%s.%s", file, holdFileExt)
imagesToHold, err := readFileLines(holdfile)
if err != nil {
return err
}

if verbose {
log.Printf("Found %d image%s to hold/ignore in %s", len(imagesToHold), pluralise(len(imagesToHold)), holdfile)
}

filtered = removeHoldImages(filtered, imagesToHold)
if verbose {
log.Printf("Processing %d image%s\n", len(filtered), pluralise(len(filtered)))
}

wg := sync.WaitGroup{}
Expand Down Expand Up @@ -128,8 +144,8 @@ Otherwise, it returns a non-zero exit code and the updated values.yaml file.`,
}()
}

for k := range filtered {
workChan <- k
for _, v := range filtered {
workChan <- v
}

close(workChan)
Expand All @@ -155,7 +171,7 @@ Otherwise, it returns a non-zero exit code and the updated values.yaml file.`,
if err := os.WriteFile(file, []byte(rawValues), 0600); err != nil {
return err
}
log.Printf("Wrote %d updates to: %s", len(updatedImages), file)
log.Printf("Wrote %d update%s to: %s", len(updatedImages), pluralise(len(updatedImages)), file)
}

return nil
Expand Down Expand Up @@ -252,3 +268,50 @@ func getTagAttributes(t string) tagAttributes {
original: t,
}
}

func readFileLines(filename string) ([]string, error) {

if _, err := os.Stat(filename); os.IsNotExist(err) {
return nil, nil
}

file, err := os.Open(filename)
if err != nil {
return nil, fmt.Errorf("failed to open file: %w", err)
}
defer file.Close()

var lines []string
scanner := bufio.NewScanner(file)
for scanner.Scan() {
lines = append(lines, scanner.Text())
}

if err := scanner.Err(); err != nil {
return nil, fmt.Errorf("error reading file: %w", err)
}

return lines, nil
}

func removeHoldImages(fullset map[string]string, held []string) map[string]string {

for _, h := range held {
serviceName := strings.TrimSuffix(h, ".image")
for k := range fullset {
if strings.EqualFold(serviceName, k) {
delete(fullset, k)
}
}
}

return fullset
}

func pluralise(count int) string {

if count == 1 {
return ""
}
return "s"
}
129 changes: 129 additions & 0 deletions cmd/chart/upgrade_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package chart

import (
"reflect"
"testing"
)

Expand Down Expand Up @@ -372,3 +373,131 @@ func TestGetTagAttributes(t *testing.T) {
})
}
}

func TestRemoveHoldImages(t *testing.T) {
tests := []struct {
name string
fullset map[string]string
held []string
expected map[string]string
}{
{
name: "Basic exclusion",
fullset: map[string]string{
"going": "registry/img1:16",
"staying1": "registry/img1:17",
"staying2": "registry/img1:18",
},
held: []string{
"going.image",
},
expected: map[string]string{
"staying1": "registry/img1:17",
"staying2": "registry/img1:18",
},
},
{
name: "Basic exclusion / muli-match",
fullset: map[string]string{
"going1": "registry/img1:16",
"going2": "registry/img1:17",
"staying": "registry/img1:18",
},
held: []string{
"going1.image",
"going2.image",
},
expected: map[string]string{
"staying": "registry/img1:18",
},
},
{
name: "No match",
fullset: map[string]string{
"staying1": "registry/img1:17",
"staying2": "registry/img1:18",
"staying3": "registry/img1:19",
},
held: []string{
"going.image",
},
expected: map[string]string{
"staying1": "registry/img1:17",
"staying2": "registry/img1:18",
"staying3": "registry/img1:19",
},
},
{
name: "Empty fullset",
fullset: map[string]string{},
held: []string{
"hold",
},
expected: map[string]string{},
},
}

for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
result := removeHoldImages(tc.fullset, tc.held)
if !reflect.DeepEqual(result, tc.expected) {
t.Errorf("\n%s \n got = %v\n want = %v", tc.name, result, tc.expected)
}
})
}
}

func TestPluralise(t *testing.T) {
tests := []struct {
name string
count int
expected string
failTest bool
}{
{
name: "Singular (1)",
count: 1,
expected: "",
failTest: false,
},
{
name: "Plural (2)",
count: 2,
expected: "s",
failTest: false,
},
{
name: "Zero",
count: 0,
expected: "s",
failTest: false,
},
{
name: "Negative count",
count: -1,
expected: "s",
failTest: false,
},
{
name: "Large plural",
count: 100,
expected: "s",
failTest: false,
},
{
name: "Large plural wrong expected",
count: 100,
expected: "",
failTest: true,
},
}

for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
result := pluralise(tc.count)
if result != tc.expected && !tc.failTest {
t.Errorf("\n%s \n got = %v\n want = %v", tc.name, result, tc.expected)
}
})
}
}
2 changes: 1 addition & 1 deletion cmd/chart/verify.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ autoscaler ghcr.io/openfaasltd/autoscaler:0.2.5
return err
}

filtered := helm.FilterImagesUptoDepth(values, depth)
filtered := helm.FilterImagesUptoDepth(values, depth, 0, "")
if len(filtered) == 0 {
return fmt.Errorf("no images found in %s", file)
}
Expand Down
10 changes: 7 additions & 3 deletions pkg/helm/io.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,18 +59,22 @@ func ReplaceValuesInHelmValuesFile(values map[string]string, yamlPath string) (s

// FilterImagesUptoDepth takes a ValuesMap and returns a map of images that
// were found upto max level
func FilterImagesUptoDepth(values ValuesMap, depth int) map[string]string {
func FilterImagesUptoDepth(values ValuesMap, depth int, level int, component string) map[string]string {
images := map[string]string{}

for k, v := range values {

if level == 1 {
component = k
}

if k == "image" && reflect.TypeOf(v).Kind() == reflect.String {
imageUrl := v.(string)
images[imageUrl] = imageUrl
images[component] = imageUrl
}

if c, ok := v.(ValuesMap); ok && depth > 0 {
images = mergeMaps(images, FilterImagesUptoDepth(c, depth-1))
images = mergeMaps(images, FilterImagesUptoDepth(c, depth-1, level+1, component))
}
}
return images
Expand Down