Skip to content

Commit

Permalink
[main] Make retrieval of tasks in bulk instead of small increments (#…
Browse files Browse the repository at this point in the history 
…3275)

* Make retrieval of tasks in bulk instead of small increments

Signed-off-by: João Pereira <joao.pereira@broadcom.com>

* Address PR comments

- Fix issue, where code was always doing 2 requests even when it was
  able to get all the tasks in the first request

Signed-off-by: João Pereira <joao.pereira@broadcom.com>

* Fix lack of output

Signed-off-by: João Pereira <joao.pereira@broadcom.com>

* Remove extra tasks from first request

Signed-off-by: João Pereira <joao.pereira@broadcom.com>

* Hardcode version because no tag exists for main branch

Signed-off-by: João Pereira <joao.pereira@broadcom.com>

* Change approach by providing the query parameters always to the request

Signed-off-by: João Pereira <joao.pereira@broadcom.com>

---------

Signed-off-by: João Pereira <joao.pereira@broadcom.com>
  • Loading branch information
@joaopapereira
joaopapereira authored Nov 6, 2024
1 parent dd2e812 commit 971e960
Show file tree
Hide file tree
Showing 5 changed files with 186 additions and 12 deletions.
4 changes: 3 additions & 1 deletion Makefile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@ FLAKE_ATTEMPTS ?=5
PACKAGES ?= api actor command types util version integration/helpers
LC_ALL = "en_US.UTF-8"

CF_BUILD_VERSION ?= $$(git describe --tags --abbrev=0)
## TODO: Change when new version is released
CF_BUILD_VERSION ?= v9.0.0
#CF_BUILD_VERSION ?= $$(git describe --tags --abbrev=0)
CF_BUILD_SHA ?= $$(git rev-parse --short HEAD)
CF_BUILD_DATE ?= $$(date -u +"%Y-%m-%d")
LD_FLAGS_COMMON=-w -s \
Expand Down
11 changes: 11 additions & 0 deletions api/cloudcontroller/ccv3/included_resources.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,14 @@ type IncludedResources struct {
ServicePlans []resources.ServicePlan `json:"service_plans,omitempty"`
Apps []resources.Application `json:"apps,omitempty"`
}

func (i *IncludedResources) Merge(resources IncludedResources) {
i.Apps = append(i.Apps, resources.Apps...)
i.Users = append(i.Users, resources.Users...)
i.Organizations = append(i.Organizations, resources.Organizations...)
i.Spaces = append(i.Spaces, resources.Spaces...)
i.ServiceBrokers = append(i.ServiceBrokers, resources.ServiceBrokers...)
i.ServiceInstances = append(i.ServiceInstances, resources.ServiceInstances...)
i.ServiceOfferings = append(i.ServiceOfferings, resources.ServiceOfferings...)
i.ServicePlans = append(i.ServicePlans, resources.ServicePlans...)
}
9 changes: 1 addition & 8 deletions api/cloudcontroller/ccv3/paginate.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,7 @@ func (requester RealRequester) paginate(request *cloudcontroller.Request, obj in
return IncludedResources{}, fullWarningsList, err
}

includes.Apps = append(includes.Apps, wrapper.IncludedResources.Apps...)
includes.Users = append(includes.Users, wrapper.IncludedResources.Users...)
includes.Organizations = append(includes.Organizations, wrapper.IncludedResources.Organizations...)
includes.Spaces = append(includes.Spaces, wrapper.IncludedResources.Spaces...)
includes.ServiceBrokers = append(includes.ServiceBrokers, wrapper.IncludedResources.ServiceBrokers...)
includes.ServiceInstances = append(includes.ServiceInstances, wrapper.IncludedResources.ServiceInstances...)
includes.ServiceOfferings = append(includes.ServiceOfferings, wrapper.IncludedResources.ServiceOfferings...)
includes.ServicePlans = append(includes.ServicePlans, wrapper.IncludedResources.ServicePlans...)
includes.Merge(wrapper.IncludedResources)

if specificPage || wrapper.NextPage() == "" {
break
Expand Down
10 changes: 10 additions & 0 deletions api/cloudcontroller/ccv3/task.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,16 @@ func (client *Client) CreateApplicationTask(appGUID string, task resources.Task)
func (client *Client) GetApplicationTasks(appGUID string, query ...Query) ([]resources.Task, Warnings, error) {
var tasks []resources.Task

foundPerPageQuery := false
for _, keyVal := range query {
if keyVal.Key == PerPage {
foundPerPageQuery = true
}
}
if !foundPerPageQuery {
query = append(query, Query{Key: PerPage, Values: []string{MaxPerPage}})
}

_, warnings, err := client.MakeListRequest(RequestParams{
RequestName: internal.GetApplicationTasksRequest,
URIParams: internal.Params{"app_guid": appGUID},
Expand Down
164 changes: 161 additions & 3 deletions api/cloudcontroller/ccv3/task_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -190,9 +190,18 @@ var _ = Describe("Task", func() {
warnings Warnings
executeErr error
)
BeforeEach(func() {
// This is required because ginkgo does not instantiate variable per test context so the tests pollute the
// variables for the next tests.
submitQuery = Query{}
})

JustBeforeEach(func() {
tasks, warnings, executeErr = client.GetApplicationTasks("some-app-guid", submitQuery)
if submitQuery.Key == "" {
tasks, warnings, executeErr = client.GetApplicationTasks("some-app-guid")
} else {
tasks, warnings, executeErr = client.GetApplicationTasks("some-app-guid", submitQuery)
}
})

When("the application exists", func() {
Expand All @@ -201,7 +210,8 @@ var _ = Describe("Task", func() {
"pagination": {
"next": {
"href": "%s/v3/apps/some-app-guid/tasks?per_page=2&page=2"
}
},
"total_results": 3
},
"resources": [
{
Expand Down Expand Up @@ -245,7 +255,7 @@ var _ = Describe("Task", func() {
)
server.AppendHandlers(
CombineHandlers(
VerifyRequest(http.MethodGet, "/v3/apps/some-app-guid/tasks", "per_page=2&page=2"),
VerifyRequest(http.MethodGet, "/v3/apps/some-app-guid/tasks", "page=2&per_page=2"),
RespondWith(http.StatusOK, response2, http.Header{"X-Cf-Warnings": {"warning-2"}}),
),
)
Expand Down Expand Up @@ -353,6 +363,154 @@ var _ = Describe("Task", func() {
Expect(warnings).To(ConsistOf("warning"))
})
})

When("the application has 10000 tasks", func() {
BeforeEach(func() {
response2 := fmt.Sprintf(`{
"pagination": {
"next": {
"href": "%s/v3/apps/some-app-guid/tasks?per_page=5000&page=2"
},
"total_results": 10000
},
"resources": [
{
"guid": "task-1-guid",
"sequence_id": 1,
"name": "task-1",
"command": "some-command",
"state": "SUCCEEDED",
"created_at": "2016-11-07T05:59:01Z"
},
{
"guid": "task-2-guid",
"sequence_id": 2,
"name": "task-2",
"command": "some-command",
"state": "FAILED",
"created_at": "2016-11-07T06:59:01Z"
}
]
}`, server.URL())
response3 := fmt.Sprintf(`{
"pagination": {
"total_results": 10000
},
"resources": [
{
"guid": "task-1-guid",
"sequence_id": 1,
"name": "task-1",
"command": "some-command",
"state": "SUCCEEDED",
"created_at": "2016-11-07T05:59:01Z"
},
{
"guid": "task-2-guid",
"sequence_id": 2,
"name": "task-2",
"command": "some-command",
"state": "FAILED",
"created_at": "2016-11-07T06:59:01Z"
}
]
}`)

server.AppendHandlers(
CombineHandlers(
VerifyRequest(http.MethodGet, "/v3/apps/some-app-guid/tasks", "per_page=5000"),
RespondWith(http.StatusOK, response2, http.Header{"X-Cf-Warnings": {"warning-2"}}),
),
)
server.AppendHandlers(
CombineHandlers(
VerifyRequest(http.MethodGet, "/v3/apps/some-app-guid/tasks", "page=2&per_page=5000"),
RespondWith(http.StatusOK, response3, http.Header{"X-Cf-Warnings": {"warning-2"}}),
),
)
})

It("calls CAPI 2 times", func() {
Expect(executeErr).ToNot(HaveOccurred())
})
})

When("the application has 4999 tasks", func() {
BeforeEach(func() {
response2 := fmt.Sprintf(`{
"pagination": {
"total_results": 4999
},
"resources": [
{
"guid": "task-1-guid",
"sequence_id": 1,
"name": "task-1",
"command": "some-command",
"state": "SUCCEEDED",
"created_at": "2016-11-07T05:59:01Z"
},
{
"guid": "task-2-guid",
"sequence_id": 2,
"name": "task-2",
"command": "some-command",
"state": "FAILED",
"created_at": "2016-11-07T06:59:01Z"
}
]
}`)
server.AppendHandlers(
CombineHandlers(
VerifyRequest(http.MethodGet, "/v3/apps/some-app-guid/tasks", "per_page=5000"),
RespondWith(http.StatusOK, response2, http.Header{"X-Cf-Warnings": {"warning-2"}}),
),
)
})

It("calls CAPI 2 times", func() {
Expect(executeErr).ToNot(HaveOccurred())
})
})

When("the application has 2 tasks", func() {
BeforeEach(func() {
response1 := fmt.Sprintf(`{
"pagination": {
"total_results": 2
},
"resources": [
{
"guid": "task-1-guid",
"sequence_id": 1,
"name": "task-1",
"command": "some-command",
"state": "SUCCEEDED",
"created_at": "2016-11-07T05:59:01Z"
},
{
"guid": "task-2-guid",
"sequence_id": 2,
"name": "task-2",
"command": "some-command",
"state": "FAILED",
"created_at": "2016-11-07T06:59:01Z"
}
]
}`)

server.AppendHandlers(
CombineHandlers(
VerifyRequest(http.MethodGet, "/v3/apps/some-app-guid/tasks"),
RespondWith(http.StatusAccepted, response1, http.Header{"X-Cf-Warnings": {"warning"}}),
),
)
})

It("calls CAPI 1 time", func() {
Expect(executeErr).ToNot(HaveOccurred())
})
})
})

Describe("UpdateTaskCancel", func() {
Expand Down

0 comments on commit 971e960

Please sign in to comment.