-
Notifications
You must be signed in to change notification settings - Fork 181
Add support of named and digest scylladb image reference with google cr #2491
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
operator: | ||
scyllaDBVersion: "6.2.0" | ||
scyllaDBVersion: "sha256:5b53a7c60d9f9555bb87791ff29b2e633c6f472aec00de7afaf4db1addc6d594" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's unclear what version it is. Use tag+sha format |
||
# scyllaDBEnterpriseVersionNeedingConsistentClusterManagementOverride sets enterprise version | ||
# that requires consistent_cluster_management workaround for restore. | ||
# In the future, enterprise versions should be run as a different config instance in its own run. | ||
|
@@ -13,6 +13,6 @@ operator: | |
prometheusVersion: "v2.54.1" | ||
operatorTests: | ||
scyllaDBVersions: | ||
updateFrom: "6.2.0-rc2" | ||
upgradeFrom: "6.1.2" | ||
updateFrom: "sha256:cf00b8bfcce4d27e8527cad913e83db89af842e6cfb64ff4736bce83594b33d1" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's unclear what it's being tested, use tag+sha format |
||
upgradeFrom: "sha256:0c1c6b49916230f62bdb0c2539b811b1a99d2c43d06ed810fad6f51015dea3ae" | ||
nodeSetupImage: "quay.io/scylladb/scylla-operator-images:node-setup-v0.0.3@sha256:c6b3de240cc5c884d5c617485bae35c51572cdfd39b6431d2e1f759c7d7feea1" |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
// Copyright (C) 2025 ScyllaDB | ||
|
||
package main | ||
|
||
import ( | ||
"flag" | ||
"fmt" | ||
"os" | ||
|
||
scyllasemver "github.com/scylladb/scylla-operator/pkg/semver" | ||
"k8s.io/klog/v2" | ||
) | ||
|
||
func main() { | ||
klog.InitFlags(flag.CommandLine) | ||
err := flag.Set("logtostderr", "true") | ||
if err != nil { | ||
panic(err) | ||
} | ||
defer klog.Flush() | ||
|
||
imageName := flag.String("image", "", "Image name to check") | ||
version := flag.String("version", "", "Version to check") | ||
|
||
flag.Parse() | ||
|
||
if *imageName == "" || *version == "" { | ||
klog.Fatalf("Usage: %s --image <imageName> --version <version>", os.Args[0]) | ||
} | ||
|
||
semVersion, fullVersion, err := scyllasemver.GetImageVersionAndDigest(*imageName, *version) | ||
if err != nil { | ||
klog.Fatalf("Error getting image version: %v", err) | ||
} | ||
|
||
fmt.Printf("%s %s\n", semVersion, fullVersion) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,11 +7,13 @@ import ( | |
"regexp" | ||
"strings" | ||
|
||
"github.com/blang/semver" | ||
"github.com/onsi/ginkgo/v2" | ||
configassets "github.com/scylladb/scylla-operator/assets/config" | ||
scyllav1 "github.com/scylladb/scylla-operator/pkg/api/scylla/v1" | ||
"github.com/scylladb/scylla-operator/pkg/genericclioptions" | ||
"github.com/scylladb/scylla-operator/pkg/helpers/slices" | ||
scyllasemver "github.com/scylladb/scylla-operator/pkg/semver" | ||
"github.com/scylladb/scylla-operator/test/e2e/framework" | ||
"github.com/spf13/cobra" | ||
apierrors "k8s.io/apimachinery/pkg/util/errors" | ||
|
@@ -27,7 +29,7 @@ const ( | |
digestRegexp = `[A-Za-z][A-Za-z0-9]*(?:[-_+.][A-Za-z][A-Za-z0-9]*)*[:][[:xdigit:]]{32,}` | ||
) | ||
|
||
var tagWithOptionalDigestRegexp = regexp.MustCompile("^" + referenceTagRegexp + "(?:@" + digestRegexp + ")?$") | ||
var tagOrDigestRegexp = regexp.MustCompile("^(?:" + referenceTagRegexp + "(?:@" + digestRegexp + ")?|" + digestRegexp + ")$") | ||
|
||
type IngressControllerOptions struct { | ||
Address string | ||
|
@@ -191,37 +193,65 @@ func (o *TestFrameworkOptions) Validate(args []string) error { | |
errors = append(errors, fmt.Errorf("gcs-service-account-key-path and s3-credentials-file-path can't be set simultanously")) | ||
} | ||
|
||
if !tagWithOptionalDigestRegexp.MatchString(o.ScyllaDBVersion) { | ||
if !tagOrDigestRegexp.MatchString(o.ScyllaDBVersion) { | ||
errors = append(errors, fmt.Errorf( | ||
"invalid scylladb-version format: %q. Expected format: <tag>[@<digest>]", | ||
"invalid scylladb-version format: %q. Expected format: <tag>, <tag>@<digest>, or <digest>", | ||
o.ScyllaDBVersion, | ||
)) | ||
} | ||
|
||
if !tagWithOptionalDigestRegexp.MatchString(o.ScyllaDBUpdateFrom) { | ||
if _, err := semver.Parse(o.ScyllaDBVersion); err != nil { | ||
semScyllaDBVersion, fullScyllaDBVersion, err := scyllasemver.GetImageVersionAndDigest("scylla", o.ScyllaDBVersion) | ||
if err != nil { | ||
return fmt.Errorf("failed to resolve ScyllaDB version: %w", err) | ||
} | ||
fmt.Println("Extracted", semScyllaDBVersion, "from ScyllaDB version:", fullScyllaDBVersion) | ||
|
||
o.ScyllaDBVersion = semScyllaDBVersion | ||
} | ||
|
||
if !tagOrDigestRegexp.MatchString(o.ScyllaDBUpdateFrom) { | ||
Comment on lines
+203
to
+213
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't all of this be part of Operator not the tests? |
||
errors = append(errors, fmt.Errorf( | ||
"invalid scylladb-update-from-version format: %q. Expected format: <tag>[@<digest>]", | ||
"invalid scylladb-update-from-version format: %q. Expected format: <tag>, <tag>@<digest>, or <digest>", | ||
o.ScyllaDBUpdateFrom, | ||
)) | ||
} | ||
|
||
if !tagWithOptionalDigestRegexp.MatchString(o.ScyllaDBUpgradeFrom) { | ||
if _, err := semver.Parse(o.ScyllaDBUpdateFrom); err != nil { | ||
semScyllaDBUpdateFromVersion, fullScyllaDBUpdateFromVersion, err := scyllasemver.GetImageVersionAndDigest("scylla", o.ScyllaDBUpdateFrom) | ||
if err != nil { | ||
return fmt.Errorf("failed to resolve ScyllaDB update from version: %w", err) | ||
} | ||
fmt.Println("Extracted", semScyllaDBUpdateFromVersion, "from ScyllaDB 'update from' version:", fullScyllaDBUpdateFromVersion) | ||
o.ScyllaDBUpdateFrom = semScyllaDBUpdateFromVersion | ||
} | ||
|
||
if !tagOrDigestRegexp.MatchString(o.ScyllaDBUpgradeFrom) { | ||
errors = append(errors, fmt.Errorf( | ||
"invalid scylladb-upgrade-from-version format: %q. Expected format: <tag>[@<digest>]", | ||
"invalid scylladb-upgrade-from-version format: %q. Expected format: <tag>, <tag>@<digest>, or <digest>", | ||
o.ScyllaDBUpgradeFrom, | ||
)) | ||
} | ||
|
||
if !tagWithOptionalDigestRegexp.MatchString(o.ScyllaDBManagerVersion) { | ||
if _, err := semver.Parse(o.ScyllaDBUpgradeFrom); err != nil { | ||
semScyllaDBUpgradeFromVersion, fullScyllaDBUpgradeFromVersion, err := scyllasemver.GetImageVersionAndDigest("scylla", o.ScyllaDBUpgradeFrom) | ||
if err != nil { | ||
return fmt.Errorf("failed to resolve ScyllaDB upgrade from version: %w", err) | ||
} | ||
fmt.Println("Extracted", semScyllaDBUpgradeFromVersion, "from ScyllaDB 'upgrade from' version:", fullScyllaDBUpgradeFromVersion) | ||
o.ScyllaDBUpgradeFrom = semScyllaDBUpgradeFromVersion | ||
} | ||
|
||
if !tagOrDigestRegexp.MatchString(o.ScyllaDBManagerVersion) { | ||
errors = append(errors, fmt.Errorf( | ||
"invalid scylladb-manager-version format: %q. Expected format: <tag>[@<digest>]", | ||
"invalid scylladb-manager-version format: %q. Expected format: <tag>, <tag>@<digest>, or <digest>", | ||
o.ScyllaDBManagerVersion, | ||
)) | ||
} | ||
|
||
if !tagWithOptionalDigestRegexp.MatchString(o.ScyllaDBManagerAgentVersion) { | ||
if !tagOrDigestRegexp.MatchString(o.ScyllaDBManagerAgentVersion) { | ||
errors = append(errors, fmt.Errorf( | ||
"invalid scylladb-manager-agent-version format: %q. Expected format: <tag>[@<digest>]", | ||
"invalid scylladb-manager-agent-version format: %q. Expected format: <tag>, <tag>@<digest>, or <digest>", | ||
o.ScyllaDBManagerAgentVersion, | ||
)) | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would you mind elaborating why this is changed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When we execute
make update
command it is updating the manifests. Without this change the ScyllaDB version is taken directly from config.yaml file which at this moment I changed to digest. So in manifest we will something like below which won't work.Note: Just to clarify. My solution is not changing almost any logic directly in operator. I just added logic to handle digest and convert it to 'supported' version before it reaches the operator/manifests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lets do other way around. Having manipulations in tests and scripts doesn't bring any value to the user. If Operator would support all valid formats of image references, then any transformation in test/scripts wouldn't be needed, and users could use all variants of image references.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think as part of this feature, we could also restructure config.yaml to accept full Scylla image ref instead just version. Places where split between repository and other part is required would be handled by the scripts around assets.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I thought the goal of this change was to allow us to use the digest/tag/'latest', so that we have more control over the exact version we're testing, rather than to please the users.