Skip to content

Commit

Permalink
Allow multiple apps in manifests
Browse files Browse the repository at this point in the history
And fix the behaviour of creating a web process during manifest
application or app creation to match cf on VMs

Issue: #1661
Co-authored-by: Giuseppe Capizzi <gcapizzi@vmware.com>
  • Loading branch information
Kieron Browne and gcapizzi committed Jan 30, 2023
1 parent 33da0ad commit 114fe00
Show file tree
Hide file tree
Showing 12 changed files with 219 additions and 93 deletions.
19 changes: 12 additions & 7 deletions api/actions/manifest.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,20 +45,25 @@ func NewManifest(domainRepo shared.CFDomainRepository, defaultDomainName string,
}
}

func (a *Manifest) Apply(ctx context.Context, authInfo authorization.Info, spaceGUID string, manifest payloads.Manifest) error {
func (a *Manifest) Apply(ctx context.Context, authInfo authorization.Info, spaceGUID string, manifesto payloads.Manifest) error {
err := a.ensureDefaultDomainConfigured(ctx, authInfo)
if err != nil {
return err
}

appInfo := manifest.Applications[0]
appState, err := a.stateCollector.CollectState(ctx, authInfo, appInfo.Name, spaceGUID)
if err != nil {
return err
for _, appInfo := range manifesto.Applications {
appState, err := a.stateCollector.CollectState(ctx, authInfo, appInfo.Name, spaceGUID)
if err != nil {
return err
}
appInfo = a.normalizer.Normalize(appInfo, appState)
err = a.applier.Apply(ctx, authInfo, spaceGUID, appInfo, appState)
if err != nil {
return err
}
}
appInfo = a.normalizer.Normalize(appInfo, appState)

return a.applier.Apply(ctx, authInfo, spaceGUID, appInfo, appState)
return nil
}

func (a *Manifest) ensureDefaultDomainConfigured(ctx context.Context, authInfo authorization.Info) error {
Expand Down
9 changes: 4 additions & 5 deletions api/actions/manifest/normalizer.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,15 +73,14 @@ func (n Normalizer) normalizeProcesses(appInfo payloads.ManifestApplication, app
break
}
}
if webProc == nil {
processes = append(processes, payloads.ManifestApplicationProcess{Type: korifiv1alpha1.ProcessTypeWeb})
webProc = &processes[len(processes)-1]
}

if appInfo.Memory != nil || appInfo.DiskQuota != nil || appInfo.Instances != nil || appInfo.Command != nil ||
appInfo.HealthCheckHTTPEndpoint != nil || appInfo.HealthCheckType != nil || appInfo.HealthCheckInvocationTimeout != nil || appInfo.Timeout != nil {

if webProc == nil {
processes = append(processes, payloads.ManifestApplicationProcess{Type: korifiv1alpha1.ProcessTypeWeb})
webProc = &processes[len(processes)-1]
}

webProc.Memory = procValIfSet(appInfo.Memory, webProc.Memory)
webProc.DiskQuota = procValIfSet(appInfo.DiskQuota, webProc.DiskQuota)
webProc.Instances = procValIfSet(appInfo.Instances, webProc.Instances)
Expand Down
13 changes: 10 additions & 3 deletions api/actions/manifest/normalizer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,8 +117,15 @@ var _ = Describe("Normalizer", func() {
}
})

It("always creates a web process", func() {
Expect(normalizedAppInfo.Processes).To(ContainElement(payloads.ManifestApplicationProcess{Type: "web"}))
})

It("preserves existing processes", func() {
Expect(normalizedAppInfo.Processes).To(ConsistOf(payloads.ManifestApplicationProcess{Type: "bob"}))
Expect(normalizedAppInfo.Processes).To(ConsistOf(
payloads.ManifestApplicationProcess{Type: "web"},
payloads.ManifestApplicationProcess{Type: "bob"},
))
})

DescribeTable("when app-level values are provided",
Expand Down Expand Up @@ -159,7 +166,7 @@ var _ = Describe("Normalizer", func() {
Expect(webProc.Timeout).To(Equal(effective.Timeout))
},

// without an existing web process
// without an explicit web process in the manifest
Entry("app-level command only",
appParams{Command: tools.PtrTo("echo boo")}, prcParams{},
expParams{Command: tools.PtrTo("echo boo")}),
Expand Down Expand Up @@ -188,7 +195,7 @@ var _ = Describe("Normalizer", func() {
appParams{Memory: tools.PtrTo("512M"), DiskQuota: tools.PtrTo("2G")}, prcParams{},
expParams{Memory: tools.PtrTo("512M"), DiskQuota: tools.PtrTo("2G")}),

// with an existing web process without the given value set
// with an explicit web process in the manifest, but without the app-level value set
Entry("empty proc with app memory",
appParams{Memory: tools.PtrTo("512M")},
prcParams{Instances: tools.PtrTo(3)},
Expand Down
93 changes: 51 additions & 42 deletions api/actions/manifest_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,10 @@ var _ = Describe("ApplyManifest", func() {
manifestAction *actions.Manifest
applyErr error

domainRepository *reposfake.CFDomainRepository
stateCollector *fake.StateCollector
normalizer *fake.Normalizer
applier *fake.Applier
appState manifest.AppState
normalizedManifest payloads.ManifestApplication
domainRepository *reposfake.CFDomainRepository
stateCollector *fake.StateCollector
normalizer *fake.Normalizer
applier *fake.Applier

appManifest payloads.Manifest
)
Expand All @@ -37,22 +35,31 @@ var _ = Describe("ApplyManifest", func() {
normalizer = new(fake.Normalizer)
applier = new(fake.Applier)

appState = manifest.AppState{
stateCollector.CollectStateReturnsOnCall(0, manifest.AppState{
App: repositories.AppRecord{
GUID: "app-guid",
Name: "app-name",
GUID: "app1-guid",
Name: "app1",
},
}
stateCollector.CollectStateReturns(appState, nil)
}, nil)
stateCollector.CollectStateReturnsOnCall(1, manifest.AppState{
App: repositories.AppRecord{
GUID: "app2-guid",
Name: "app2",
},
}, nil)

normalizedManifest = payloads.ManifestApplication{
Name: "app-name",
}
normalizer.NormalizeReturns(normalizedManifest)
normalizer.NormalizeReturnsOnCall(0, payloads.ManifestApplication{
Name: "normalized-app1",
})
normalizer.NormalizeReturnsOnCall(1, payloads.ManifestApplication{
Name: "normalized-app2",
})

appManifest = payloads.Manifest{
Applications: []payloads.ManifestApplication{{
Name: "app-name",
Name: "app1",
}, {
Name: "app2",
}},
}

Expand All @@ -63,14 +70,38 @@ var _ = Describe("ApplyManifest", func() {
applyErr = manifestAction.Apply(context.Background(), authorization.Info{}, "space-guid", appManifest)
})

It("succeeds", func() {
It("normalizes the manifest and then applies it", func() {
Expect(applyErr).NotTo(HaveOccurred())
})

It("ensures the default domain is configured", func() {
Expect(domainRepository.GetDomainByNameCallCount()).To(Equal(1))
_, _, actualDomain := domainRepository.GetDomainByNameArgsForCall(0)
Expect(actualDomain).To(Equal("my.domain"))

Expect(stateCollector.CollectStateCallCount()).To(Equal(2))
_, _, actualAppName, actualSpaceGUID := stateCollector.CollectStateArgsForCall(0)
Expect(actualAppName).To(Equal("app1"))
Expect(actualSpaceGUID).To(Equal("space-guid"))
_, _, actualAppName, actualSpaceGUID = stateCollector.CollectStateArgsForCall(1)
Expect(actualAppName).To(Equal("app2"))
Expect(actualSpaceGUID).To(Equal("space-guid"))

Expect(normalizer.NormalizeCallCount()).To(Equal(2))
actualAppInManifest, actualState := normalizer.NormalizeArgsForCall(0)
Expect(actualAppInManifest.Name).To(Equal("app1"))
Expect(actualState.App.GUID).To(Equal("app1-guid"))
actualAppInManifest, actualState = normalizer.NormalizeArgsForCall(1)
Expect(actualAppInManifest.Name).To(Equal("app2"))
Expect(actualState.App.GUID).To(Equal("app2-guid"))

Expect(applier.ApplyCallCount()).To(Equal(2))
_, _, actualSpaceGUID, actualAppInManifest, actualState = applier.ApplyArgsForCall(0)
Expect(actualSpaceGUID).To(Equal("space-guid"))
Expect(actualAppInManifest.Name).To(Equal("normalized-app1"))
Expect(actualState.App.GUID).To(Equal("app1-guid"))
_, _, actualSpaceGUID, actualAppInManifest, actualState = applier.ApplyArgsForCall(1)
Expect(actualSpaceGUID).To(Equal("space-guid"))
Expect(actualAppInManifest.Name).To(Equal("normalized-app2"))
Expect(actualState.App.GUID).To(Equal("app2-guid"))
})

When("the default domain does not exist", func() {
Expand All @@ -93,38 +124,16 @@ var _ = Describe("ApplyManifest", func() {
})
})

It("collects the app state", func() {
Expect(stateCollector.CollectStateCallCount()).To(Equal(1))
_, _, actualAppName, actualSpaceGUID := stateCollector.CollectStateArgsForCall(0)
Expect(actualAppName).To(Equal("app-name"))
Expect(actualSpaceGUID).To(Equal("space-guid"))
})

When("collecting the app state fails", func() {
BeforeEach(func() {
stateCollector.CollectStateReturns(manifest.AppState{}, errors.New("collect-state-err"))
stateCollector.CollectStateReturnsOnCall(0, manifest.AppState{}, errors.New("collect-state-err"))
})

It("returns the error", func() {
Expect(applyErr).To(MatchError("collect-state-err"))
})
})

It("normalizes the manifest", func() {
Expect(normalizer.NormalizeCallCount()).To(Equal(1))
actualAppInManifest, actualState := normalizer.NormalizeArgsForCall(0)
Expect(actualAppInManifest.Name).To(Equal("app-name"))
Expect(actualState.App.GUID).To(Equal("app-guid"))
})

It("applies the normalized manifest", func() {
Expect(applier.ApplyCallCount()).To(Equal(1))
_, _, actualSpaceGUID, actualAppInManifest, actualState := applier.ApplyArgsForCall(0)
Expect(actualSpaceGUID).To(Equal("space-guid"))
Expect(actualAppInManifest.Name).To(Equal("app-name"))
Expect(actualState.App.GUID).To(Equal("app-guid"))
})

When("applying the normalized manifest fails", func() {
BeforeEach(func() {
applier.ApplyReturns(errors.New("apply-err"))
Expand Down
10 changes: 10 additions & 0 deletions api/handlers/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"code.cloudfoundry.org/korifi/api/presenter"
"code.cloudfoundry.org/korifi/api/repositories"
"code.cloudfoundry.org/korifi/api/routing"
korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1"
"code.cloudfoundry.org/korifi/tools"

"github.com/go-logr/logr"
Expand Down Expand Up @@ -131,6 +132,15 @@ func (h *App) create(r *http.Request) (*routing.Response, error) {
return nil, apierrors.LogAndReturn(logger, err, "Failed to create app", "App Name", payload.Name)
}

err = h.processRepo.CreateProcess(r.Context(), authInfo, repositories.CreateProcessMessage{
AppGUID: appRecord.GUID,
SpaceGUID: spaceGUID,
Type: korifiv1alpha1.ProcessTypeWeb,
})
if err != nil {
return nil, apierrors.LogAndReturn(logger, err, "Failed to create web process", "App Name", payload.Name)
}

return routing.NewResponse(http.StatusCreated).WithBody(presenter.ForApp(appRecord, h.serverURL)), nil
}

Expand Down
21 changes: 21 additions & 0 deletions api/handlers/app_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,27 @@ var _ = Describe("App", func() {
}`, defaultServerURL, appGUID, spaceGUID)), "Response body matches response:")
})

It("creates the `web` process", func() {
Expect(processRepo.CreateProcessCallCount()).To(Equal(1))
_, actualAuthInfo, actualMsg := processRepo.CreateProcessArgsForCall(0)
Expect(actualAuthInfo).To(Equal(authInfo))
Expect(actualMsg).To(Equal(repositories.CreateProcessMessage{
AppGUID: appGUID,
SpaceGUID: spaceGUID,
Type: "web",
}))
})

When("creating the process fails", func() {
BeforeEach(func() {
processRepo.CreateProcessReturns(errors.New("create-process-err"))
})

It("returns an error", func() {
expectUnknownError()
})
})

When("creating the app fails", func() {
BeforeEach(func() {
appRepo.CreateAppReturns(repositories.AppRecord{}, errors.New("create-app-err"))
Expand Down
78 changes: 78 additions & 0 deletions api/handlers/fake/cfprocess_repository.go

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

1 change: 1 addition & 0 deletions api/handlers/process.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ type CFProcessRepository interface {
ListProcesses(context.Context, authorization.Info, repositories.ListProcessesMessage) ([]repositories.ProcessRecord, error)
GetProcessByAppTypeAndSpace(context.Context, authorization.Info, string, string, string) (repositories.ProcessRecord, error)
PatchProcess(context.Context, authorization.Info, repositories.PatchProcessMessage) (repositories.ProcessRecord, error)
CreateProcess(context.Context, authorization.Info, repositories.CreateProcessMessage) error
}

//counterfeiter:generate -o fake -fake-name ProcessScaler . ProcessScaler
Expand Down
2 changes: 1 addition & 1 deletion api/payloads/manifest.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import (

type Manifest struct {
Version int `yaml:"version"`
Applications []ManifestApplication `yaml:"applications" validate:"max=1,dive"`
Applications []ManifestApplication `yaml:"applications"`
}

type ManifestApplication struct {
Expand Down
Loading

0 comments on commit 114fe00

Please sign in to comment.