Skip to content

Commit

Permalink
Part 1: Reduce number of variable sources. Bundle uniqueness (#497)
Browse files Browse the repository at this point in the history 
* Move files

Signed-off-by: Mikalai Radchuk <[email protected]>

* Move bundle uniqueness code into a func

Signed-off-by: Mikalai Radchuk <[email protected]>

---------

Signed-off-by: Mikalai Radchuk <[email protected]>
  • Loading branch information
@m1kola
m1kola authored Nov 2, 2023
1 parent e78fd95 commit 9c7291d
Show file tree
Hide file tree
Showing 3 changed files with 143 additions and 30 deletions.
2 changes: 1 addition & 1 deletion go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ require (
github.com/Masterminds/semver/v3 v3.2.1
github.com/blang/semver/v4 v4.0.0
github.com/go-logr/logr v1.3.0
github.com/google/go-cmp v0.6.0
github.com/onsi/ginkgo/v2 v2.13.0
github.com/onsi/gomega v1.29.0
github.com/operator-framework/catalogd v0.7.0
Expand Down Expand Up @@ -70,7 +71,6 @@ require (
github.com/golang/protobuf v1.5.3 // indirect
github.com/google/cel-go v0.12.6 // indirect
github.com/google/gnostic v0.5.7-v3refs // indirect
github.com/google/go-cmp v0.6.0 // indirect
github.com/google/gofuzz v1.2.0 // indirect
github.com/google/pprof v0.0.0-20210720184732-4bb14d4b1be1 // indirect
github.com/google/uuid v1.3.0 // indirect
Expand Down
74 changes: 45 additions & 29 deletions ...lution/variablesources/crd_constraints.go → ...tion/variablesources/bundle_uniqueness.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,53 @@ import (
"context"
"fmt"

"k8s.io/apimachinery/pkg/util/sets"

"github.com/operator-framework/deppy/pkg/deppy"
"github.com/operator-framework/deppy/pkg/deppy/input"
"k8s.io/apimachinery/pkg/util/sets"

"github.com/operator-framework/operator-controller/internal/catalogmetadata"
olmvariables "github.com/operator-framework/operator-controller/internal/resolution/variables"
)

var _ input.VariableSource = &CRDUniquenessConstraintsVariableSource{}
// MakeBundleUniquenessVariables produces variables that constrain
// the solution to at most 1 bundle per package.
// These variables guarantee that no two versions of
// the same package are running at the same time.
func MakeBundleUniquenessVariables(bundleVariables []*olmvariables.BundleVariable) []*olmvariables.BundleUniquenessVariable {
result := []*olmvariables.BundleUniquenessVariable{}

bundleIDs := sets.Set[deppy.Identifier]{}
packageOrder := []string{}
bundleOrder := map[string][]deppy.Identifier{}
for _, bundleVariable := range bundleVariables {
bundles := make([]*catalogmetadata.Bundle, 0, 1+len(bundleVariable.Dependencies()))
bundles = append(bundles, bundleVariable.Bundle())
bundles = append(bundles, bundleVariable.Dependencies()...)
for _, bundle := range bundles {
id := olmvariables.BundleVariableID(bundle)
// get bundleID package and update map
packageName := bundle.Package

if _, ok := bundleOrder[packageName]; !ok {
packageOrder = append(packageOrder, packageName)
}

if !bundleIDs.Has(id) {
bundleIDs.Insert(id)
bundleOrder[packageName] = append(bundleOrder[packageName], id)
}
}
}

// create global constraint variables
for _, packageName := range packageOrder {
varID := deppy.IdentifierFromString(fmt.Sprintf("%s package uniqueness", packageName))
result = append(result, olmvariables.NewBundleUniquenessVariable(varID, bundleOrder[packageName]...))
}

return result
}

// CRDUniquenessConstraintsVariableSource produces variables that constraint the solution to
// 1. at most 1 bundle per package
Expand Down Expand Up @@ -41,39 +79,17 @@ func (g *CRDUniquenessConstraintsVariableSource) GetVariables(ctx context.Contex
return nil, err
}

// todo(perdasilva): better handle cases where a provided gvk is not found
// not all packages will necessarily export a CRD

bundleIDs := sets.Set[deppy.Identifier]{}
packageOrder := []string{}
bundleOrder := map[string][]deppy.Identifier{}
bundleVariables := []*olmvariables.BundleVariable{}
for _, variable := range variables {
switch v := variable.(type) {
case *olmvariables.BundleVariable:
bundles := []*catalogmetadata.Bundle{v.Bundle()}
bundles = append(bundles, v.Dependencies()...)
for _, bundle := range bundles {
id := olmvariables.BundleVariableID(bundle)
// get bundleID package and update map
packageName := bundle.Package

if _, ok := bundleOrder[packageName]; !ok {
packageOrder = append(packageOrder, packageName)
}

if !bundleIDs.Has(id) {
bundleIDs.Insert(id)
bundleOrder[packageName] = append(bundleOrder[packageName], id)
}
}
bundleVariables = append(bundleVariables, v)
}
}

// create global constraint variables
for _, packageName := range packageOrder {
varID := deppy.IdentifierFromString(fmt.Sprintf("%s package uniqueness", packageName))
variables = append(variables, olmvariables.NewBundleUniquenessVariable(varID, bundleOrder[packageName]...))
bundleUniqueness := MakeBundleUniquenessVariables(bundleVariables)
for _, v := range bundleUniqueness {
variables = append(variables, v)
}

return variables, nil
}
97 changes: 97 additions & 0 deletions ...n/variablesources/crd_constraints_test.go → ...variablesources/bundle_uniqueness_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,16 @@ import (
"context"
"encoding/json"
"fmt"
"testing"

"github.com/google/go-cmp/cmp"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
"github.com/stretchr/testify/require"

"github.com/operator-framework/deppy/pkg/deppy"
"github.com/operator-framework/deppy/pkg/deppy/constraint"
"github.com/operator-framework/deppy/pkg/deppy/input"
"github.com/operator-framework/operator-registry/alpha/declcfg"
"github.com/operator-framework/operator-registry/alpha/property"

Expand All @@ -17,6 +22,98 @@ import (
"github.com/operator-framework/operator-controller/internal/resolution/variablesources"
)

func TestMakeBundleUniquenessVariables(t *testing.T) {
const fakeCatalogName = "fake-catalog"
channel := catalogmetadata.Channel{Channel: declcfg.Channel{Name: "stable"}}
bundleSet := map[string]*catalogmetadata.Bundle{
"test-package.v1.0.0": {
Bundle: declcfg.Bundle{
Name: "test-package.v1.0.0",
Package: "test-package",
Properties: []property.Property{
{Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "1.0.0"}`)},
{Type: property.TypePackageRequired, Value: json.RawMessage(`{"packageName": "some-package", "versionRange": ">=1.0.0 <2.0.0"}`)},
},
},
CatalogName: fakeCatalogName,
InChannels: []*catalogmetadata.Channel{&channel},
},
"test-package.v1.0.1": {
Bundle: declcfg.Bundle{
Name: "test-package.v1.0.1",
Package: "test-package",
Properties: []property.Property{
{Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "1.0.1"}`)},
{Type: property.TypePackageRequired, Value: json.RawMessage(`{"packageName": "some-package", "versionRange": ">=1.0.0 <2.0.0"}`)},
},
},
CatalogName: fakeCatalogName,
InChannels: []*catalogmetadata.Channel{&channel},
},

"some-package.v1.0.0": {
Bundle: declcfg.Bundle{
Name: "some-package.v1.0.0",
Package: "some-package",
Properties: []property.Property{
{Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "some-package", "version": "1.0.0"}`)},
},
},
CatalogName: fakeCatalogName,
InChannels: []*catalogmetadata.Channel{&channel},
},
}

// Input into the testable function must include more than one bundle
// from the same package to ensure that the function
// enforces uniqueness correctly.
// We also need at least one bundle variable to have a dependency
// on another package. This will help to make sure that the function
// also creates uniqueness variables for dependencies.
bundleVariables := []*olmvariables.BundleVariable{
olmvariables.NewBundleVariable(
bundleSet["test-package.v1.0.0"],
[]*catalogmetadata.Bundle{
bundleSet["some-package.v1.0.0"],
},
),
olmvariables.NewBundleVariable(
bundleSet["test-package.v1.0.1"],
[]*catalogmetadata.Bundle{
bundleSet["some-package.v1.0.0"],
},
),
}

variables := variablesources.MakeBundleUniquenessVariables(bundleVariables)

// Each package in the input must have own uniqueness variable.
// Each variable expected to have one constraint enforcing at most one
// of the involved bundles to be allowed in the solution
expectedVariables := []*olmvariables.BundleUniquenessVariable{
{
SimpleVariable: input.NewSimpleVariable(
"test-package package uniqueness",
constraint.AtMost(
1,
deppy.Identifier("fake-catalog-test-package-test-package.v1.0.0"),
deppy.Identifier("fake-catalog-test-package-test-package.v1.0.1"),
),
),
},
{
SimpleVariable: input.NewSimpleVariable(
"some-package package uniqueness",
constraint.AtMost(
1,
deppy.Identifier("fake-catalog-some-package-some-package.v1.0.0"),
),
),
},
}
require.Empty(t, cmp.Diff(variables, expectedVariables, cmp.AllowUnexported(input.SimpleVariable{}, constraint.AtMostConstraint{})))
}

var channel = catalogmetadata.Channel{Channel: declcfg.Channel{Name: "stable"}}
var bundleSet = map[string]*catalogmetadata.Bundle{
// required package bundles
Expand Down

0 comments on commit 9c7291d

Please sign in to comment.