Skip to content
This repository has been archived by the owner on Oct 9, 2023. It is now read-only.

Commit

Permalink
Refactor the resources model of FlyteAdmin (#54)
Browse files Browse the repository at this point in the history
* Refactor the resources model of FlyteAdmin
* Doing integration test and pushing fixes
  • Loading branch information
anandswaminathan authored Jan 21, 2020
1 parent b80c3de commit 6815fba
Show file tree
Hide file tree
Showing 65 changed files with 1,602 additions and 2,589 deletions.
39 changes: 20 additions & 19 deletions flyteadmin/Gopkg.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 6 additions & 6 deletions flyteadmin/Gopkg.toml
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@
[[override]]
name = "github.com/lyft/flyteidl"
source = "https://github.com/lyft/flyteidl"
version = "=0.16.5"
version = "^0.16.6"

[[constraint]]
name = "github.com/lyft/flytepropeller"
Expand Down Expand Up @@ -92,11 +92,6 @@
name = "gopkg.in/gormigrate.v1"
version = "1.2.1"

[[override]]
name = "k8s.io/apimachinery"
source = "https://github.com/lyft/apimachinery"
revision = "047e3ea32d7fb5984f444d7dd9510cfd362d7d7c"

[[override]]
name = "k8s.io/api"
revision = "b49a72c274e072a6e385d55c671acb3717186ce5"
Expand All @@ -122,6 +117,11 @@
go-tests = true
unused-packages = true

[[override]]
name = "k8s.io/apimachinery"
source = "https://github.com/lyft/apimachinery"
revision = "047e3ea32d7fb5984f444d7dd9510cfd362d7d7c"

[[constraint]]
name = "github.com/graymeta/stow"
revision = "903027f87de7054953efcdb8ba70d5dc02df38c7"
20 changes: 11 additions & 9 deletions flyteadmin/pkg/clusterresource/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ import (
"strings"
"time"

"github.com/lyft/flyteadmin/pkg/resourcematching"
"github.com/lyft/flyteadmin/pkg/manager/impl/resources"
managerinterfaces "github.com/lyft/flyteadmin/pkg/manager/interfaces"

"github.com/lyft/flyteadmin/pkg/executioncluster/interfaces"

Expand Down Expand Up @@ -70,6 +71,7 @@ type controller struct {
db repositories.RepositoryInterface
config runtimeInterfaces.Configuration
executionCluster interfaces.ClusterInterface
resourceManager managerinterfaces.ResourceInterface
poller chan struct{}
metrics controllerMetrics
lastAppliedTemplateDir string
Expand Down Expand Up @@ -175,17 +177,16 @@ func (c *controller) getCustomTemplateValues(
}
collectedErrs := make([]error, 0)
// All override values saved in the database take precedence over the domain-specific defaults.
attributes, err := resourcematching.GetOverrideValuesToApply(ctx, resourcematching.GetOverrideValuesInput{
Db: c.db,
Project: project,
Domain: domain,
Resource: admin.MatchableResource_CLUSTER_RESOURCE,
resource, err := c.resourceManager.GetResource(ctx, managerinterfaces.ResourceRequest{
Project: project,
Domain: domain,
ResourceType: admin.MatchableResource_CLUSTER_RESOURCE,
})
if err != nil {
collectedErrs = append(collectedErrs, err)
}
if attributes != nil && attributes.GetClusterResourceAttributes() != nil {
for templateKey, templateValue := range attributes.GetClusterResourceAttributes().Attributes {
if resource != nil && resource.Attributes != nil && resource.Attributes.GetClusterResourceAttributes() != nil {
for templateKey, templateValue := range resource.Attributes.GetClusterResourceAttributes().Attributes {
customTemplateValues[fmt.Sprintf(templateVariableFormat, templateKey)] = templateValue
}
}
Expand Down Expand Up @@ -285,7 +286,7 @@ func (c *controller) syncNamespace(ctx context.Context, namespace NamespaceName,
err = target.Client.Create(ctx, k8sObjCopy)
if err != nil {
if k8serrors.IsAlreadyExists(err) {
logger.Debugf(ctx, "Resource [%+v] in namespace [%s] already exists - attempting update instead",
logger.Debugf(ctx, "Type [%+v] in namespace [%s] already exists - attempting update instead",
k8sObj.GetObjectKind().GroupVersionKind().Kind, namespace)
c.metrics.AppliedTemplateExists.Inc()
// Use a strategic-merge-patch to mimic `kubectl apply` behavior.
Expand Down Expand Up @@ -423,6 +424,7 @@ func NewClusterResourceController(db repositories.RepositoryInterface, execution
db: db,
config: config,
executionCluster: executionCluster,
resourceManager: resources.NewResourceManager(db),
poller: make(chan struct{}),
metrics: newMetrics(scope),
appliedTemplates: make(map[string]map[string]time.Time),
Expand Down
28 changes: 16 additions & 12 deletions flyteadmin/pkg/clusterresource/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ import (
"testing"
"time"

"github.com/lyft/flyteadmin/pkg/manager/impl/resources"
"github.com/lyft/flyteadmin/pkg/repositories/interfaces"

"github.com/lyft/flyteadmin/pkg/repositories/transformers"
"github.com/lyft/flyteidl/gen/pb-go/flyteidl/admin"

Expand Down Expand Up @@ -161,16 +164,16 @@ func TestGetCustomTemplateValues(t *testing.T) {
},
},
}
projectDomainModel, err := transformers.ToProjectDomainAttributesModel(projectDomainAttributes, admin.MatchableResource_CLUSTER_RESOURCE)
resourceModel, err := transformers.ProjectDomainAttributesToResourceModel(projectDomainAttributes, admin.MatchableResource_CLUSTER_RESOURCE)
assert.Nil(t, err)
mockRepository.ProjectDomainAttributesRepo().(*repositoryMocks.MockProjectDomainAttributesRepo).GetFunction = func(
ctx context.Context, project, domain, resource string) (models.ProjectDomainAttributes, error) {
assert.Equal(t, "project-foo", project)
assert.Equal(t, "domain-bar", domain)
return projectDomainModel, nil
mockRepository.ResourceRepo().(*repositoryMocks.MockResourceRepo).GetFunction = func(ctx context.Context, ID interfaces.ResourceID) (resource models.Resource, e error) {
assert.Equal(t, "project-foo", ID.Project)
assert.Equal(t, "domain-bar", ID.Domain)
return resourceModel, nil
}
testController := controller{
db: mockRepository,
db: mockRepository,
resourceManager: resources.NewResourceManager(mockRepository),
}
domainTemplateValues := templateValuesType{
"{{ var1 }}": "i'm getting overwritten",
Expand All @@ -191,7 +194,8 @@ func TestGetCustomTemplateValues(t *testing.T) {
func TestGetCustomTemplateValues_NothingToOverride(t *testing.T) {
mockRepository := repositoryMocks.NewMockRepository()
testController := controller{
db: mockRepository,
db: mockRepository,
resourceManager: resources.NewResourceManager(mockRepository),
}
customTemplateValues, err := testController.getCustomTemplateValues(context.Background(), "project-foo", "domain-bar", templateValuesType{
"{{ var1 }}": "val1",
Expand All @@ -207,14 +211,14 @@ func TestGetCustomTemplateValues_NothingToOverride(t *testing.T) {

func TestGetCustomTemplateValues_InvalidDBModel(t *testing.T) {
mockRepository := repositoryMocks.NewMockRepository()
mockRepository.ProjectDomainAttributesRepo().(*repositoryMocks.MockProjectDomainAttributesRepo).GetFunction = func(
ctx context.Context, project, domain, resource string) (models.ProjectDomainAttributes, error) {
return models.ProjectDomainAttributes{
mockRepository.ResourceRepo().(*repositoryMocks.MockResourceRepo).GetFunction = func(ctx context.Context, ID interfaces.ResourceID) (resource models.Resource, e error) {
return models.Resource{
Attributes: []byte("i'm invalid"),
}, nil
}
testController := controller{
db: mockRepository,
db: mockRepository,
resourceManager: resources.NewResourceManager(mockRepository),
}
_, err := testController.getCustomTemplateValues(context.Background(), "project-foo", "domain-bar", templateValuesType{
"{{ var1 }}": "val1",
Expand Down
Loading

0 comments on commit 6815fba

Please sign in to comment.