Skip to content
This repository was archived by the owner on Jun 2, 2021. It is now read-only.

Commit 16f59c7

Browse files
cwlbraacwlbraa
committed
polish rough edges of kpack buildpack select
- support re-pushing by replace-updating the CustomBuilder when necessary - delete the custombuilder when the app is deleted [finishes #171029528] Authored-by: Connor Braa <cbraa@pivotal.io>
1 parent 8eaf17d commit 16f59c7

18 files changed

Lines changed: 156 additions & 38 deletions

File tree

‎app/actions/app_delete.rb‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
module VCAP::CloudController
1818
class AppDelete
1919
class AsyncBindingDeletionsTriggered < StandardError; end
20+
2021
class SubResourceError < StandardError
2122
def initialize(errors)
2223
@errors = errors
@@ -53,10 +54,15 @@ def delete(apps, record_event: true)
5354
logger.info("Deleted app: #{app.guid}")
5455

5556
if VCAP::CloudController::Config.kubernetes_api_configured?
56-
logger.info('Deleting associated kpack image')
57+
# an app CRD could simplify this garbage collection
58+
logger.info("Deleting kpack resources associated with app #{app.guid}")
5759
k8s_api_client.delete_image(
5860
app.guid,
59-
VCAP::CloudController::Config.config.kpack_builder_namespace
61+
VCAP::CloudController::Config.config.kpack_builder_namespace,
62+
)
63+
k8s_api_client.delete_custom_builder(
64+
"app-#{app.guid}",
65+
VCAP::CloudController::Config.config.kpack_builder_namespace,
6066
)
6167
end
6268
end

‎lib/cloud_controller/kpack/stager.rb‎

Lines changed: 30 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
module Kpack
22
class Stager
3-
CF_DEFAULT_BUILDER_SPEC = {
3+
CF_DEFAULT_BUILDER_REFERENCE = {
44
name: 'cf-default-builder',
55
kind: 'CustomBuilder'
66
}.freeze
@@ -15,13 +15,13 @@ def initialize(builder_namespace:, registry_service_account_name:, registry_tag_
1515
end
1616

1717
def stage(staging_details)
18-
builder_spec = get_or_create_builder_spec(staging_details)
18+
builder_reference = find_or_create_builder_reference(staging_details)
1919

2020
existing_image = client.get_image(staging_details.package.app.guid, builder_namespace)
2121
if existing_image.present?
22-
client.update_image(update_image_resource(existing_image, staging_details, builder_spec))
22+
client.update_image(update_image_resource(existing_image, staging_details, builder_reference))
2323
else
24-
client.create_image(image_resource(staging_details, builder_spec))
24+
client.create_image(image_resource(staging_details, builder_reference))
2525
end
2626
rescue CloudController::Errors::ApiError => e
2727
build = VCAP::CloudController::BuildModel.find(guid: staging_details.staging_guid)
@@ -94,19 +94,36 @@ def get_environment_variables(staging_details)
9494
map { |key, value| { name: key, value: value.to_s } }
9595
end
9696

97-
def get_or_create_builder_spec(staging_details)
98-
return create_custom_builder(staging_details) if staging_details.lifecycle.buildpack_infos.present?
97+
def find_or_create_builder_reference(staging_details)
98+
return CF_DEFAULT_BUILDER_REFERENCE unless staging_details.lifecycle.buildpack_infos.present?
9999

100-
CF_DEFAULT_BUILDER_SPEC
100+
custom_builder_name = "app-#{staging_details.package.app.guid}"
101+
create_or_update_custom_builder(custom_builder_name, staging_details)
102+
103+
{
104+
name: custom_builder_name,
105+
kind: 'CustomBuilder'
106+
}
101107
end
102108

103-
def create_custom_builder(staging_details)
104-
default_builder = client.get_custom_builder(CF_DEFAULT_BUILDER_SPEC[:name], builder_namespace)
109+
def create_or_update_custom_builder(name, staging_details)
110+
existing = client.get_custom_builder(name, builder_namespace)
111+
unless existing.present?
112+
return client.create_custom_builder(generate_custom_builder_from_default(name, staging_details))
113+
end
105114

106-
custom_builder_name = "app-#{staging_details.package.app.guid}"
107-
client.create_custom_builder(Kubeclient::Resource.new({
115+
desired_custom_builder = generate_custom_builder_from_default(name, staging_details)
116+
desired_custom_builder.metadata.resourceVersion = existing.metadata.resourceVersion
117+
desired_custom_builder.apiVersion = existing.apiVersion
118+
client.update_custom_builder(desired_custom_builder)
119+
end
120+
121+
def generate_custom_builder_from_default(name, staging_details)
122+
default_builder = client.get_custom_builder(CF_DEFAULT_BUILDER_REFERENCE[:name], builder_namespace)
123+
Kubeclient::Resource.new({
124+
kind: 'CustomBuilder',
108125
metadata: {
109-
name: custom_builder_name,
126+
name: name,
110127
namespace: builder_namespace,
111128
labels: {
112129
APP_GUID_LABEL_KEY.to_sym => staging_details.package.app.guid,
@@ -123,12 +140,7 @@ def create_custom_builder(staging_details)
123140
group: staging_details.lifecycle.buildpack_infos.map { |buildpack| { id: buildpack } }
124141
]
125142
}
126-
}))
127-
128-
{
129-
name: custom_builder_name,
130-
kind: 'CustomBuilder'
131-
}
143+
})
132144
end
133145

134146
def logger

‎lib/kubernetes/api_client.rb‎

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,10 +86,33 @@ def delete_route(name, namespace)
8686
raise error
8787
end
8888

89+
def update_custom_builder(resource_config)
90+
@kpack_kube_client.update_custom_builder(resource_config)
91+
rescue Kubeclient::HttpError => e
92+
logger.error('update_custom_builder', error: e.inspect, response: e.response, backtrace: e.backtrace)
93+
error = CloudController::Errors::ApiError.new_from_details('KpackCustomBuilderError', 'update', e.message)
94+
error.set_backtrace(e.backtrace)
95+
raise error
96+
end
97+
8998
def create_custom_builder(resource_config)
9099
@kpack_kube_client.create_custom_builder(resource_config)
91100
rescue Kubeclient::HttpError => e
92-
raise CloudController::Errors::ApiError.new_from_details('KpackCustomBuilderError', 'create', e.message)
101+
logger.error('create_custom_builder', error: e.inspect, response: e.response, backtrace: e.backtrace)
102+
error = CloudController::Errors::ApiError.new_from_details('KpackCustomBuilderError', 'create', e.message)
103+
error.set_backtrace(e.backtrace)
104+
raise error
105+
end
106+
107+
def delete_custom_builder(name, namespace)
108+
@kpack_kube_client.delete_custom_builder(name, namespace)
109+
rescue Kubeclient::ResourceNotFoundError
110+
nil
111+
rescue Kubeclient::HttpError => e
112+
logger.error('delete_custom_builder', error: e.inspect, response: e.response, backtrace: e.backtrace)
113+
error = CloudController::Errors::ApiError.new_from_details('KpackCustomBuilderError', 'delete', e.message)
114+
error.set_backtrace(e.backtrace)
115+
raise error
93116
end
94117

95118
def get_custom_builder(name, namespace)

‎spec/api/documentation/apps_api_spec.rb‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ def self.fields_info(required)
122122
end
123123

124124
describe 'Standard endpoints' do
125-
let(:k8s_api_client) { instance_double(Kubernetes::ApiClient, delete_image: nil) }
125+
let(:k8s_api_client) { instance_double(Kubernetes::ApiClient, delete_image: nil, delete_custom_builder: nil) }
126126

127127
before do
128128
allow(CloudController::DependencyLocator.instance).to receive(:k8s_api_client).and_return(k8s_api_client)

‎spec/request/apps_spec.rb‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1509,7 +1509,7 @@
15091509
let!(:process) { VCAP::CloudController::ProcessModel.make(app: app_model) }
15101510
let!(:deployment) { VCAP::CloudController::DeploymentModel.make(app: app_model) }
15111511
let(:user_email) { nil }
1512-
let(:k8s_api_client) { instance_double(Kubernetes::ApiClient, delete_image: nil) }
1512+
let(:k8s_api_client) { instance_double(Kubernetes::ApiClient, delete_image: nil, delete_custom_builder: nil) }
15131513

15141514
before do
15151515
space.organization.add_user(user)

‎spec/request/builds_spec.rb‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,7 @@
142142
allow(CloudController::DependencyLocator.instance).to receive(:k8s_api_client).and_return(k8s_api_client)
143143
allow(k8s_api_client).to receive(:create_image)
144144
allow(k8s_api_client).to receive(:create_custom_builder)
145+
allow(k8s_api_client).to receive(:update_custom_builder)
145146
allow(k8s_api_client).to receive(:get_image)
146147
allow(k8s_api_client).to receive(:get_custom_builder).and_return(Kubeclient::Resource.new({
147148
metadata: {
@@ -208,7 +209,7 @@
208209
expect(parsed_response['lifecycle']['data']['buildpacks']).to eq ['paketo-buildpacks/java', 'paketo-community/ruby']
209210
expect(parsed_response['state']).to eq 'STAGING'
210211

211-
expect(k8s_api_client).to have_received(:create_custom_builder)
212+
expect(k8s_api_client).to have_received(:update_custom_builder)
212213
end
213214
end
214215

‎spec/request/organizations_spec.rb‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1122,7 +1122,7 @@ module VCAP::CloudController
11221122
s.add_shared_space(space)
11231123
s
11241124
end
1125-
let(:k8s_api_client) { instance_double(Kubernetes::ApiClient, delete_image: nil) }
1125+
let(:k8s_api_client) { instance_double(Kubernetes::ApiClient, delete_image: nil, delete_custom_builder: nil) }
11261126

11271127
before do
11281128
allow(CloudController::DependencyLocator.instance).to receive(:k8s_api_client).and_return(k8s_api_client)

‎spec/request/spaces_spec.rb‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -889,7 +889,7 @@
889889
s.add_shared_space(space)
890890
s
891891
end
892-
let(:k8s_api_client) { instance_double(Kubernetes::ApiClient, delete_image: nil) }
892+
let(:k8s_api_client) { instance_double(Kubernetes::ApiClient, delete_image: nil, delete_custom_builder: nil) }
893893

894894
before do
895895
allow(CloudController::DependencyLocator.instance).to receive(:k8s_api_client).and_return(k8s_api_client)

‎spec/request/v2/apps_spec.rb‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1127,7 +1127,7 @@
11271127

11281128
describe 'DELETE /v2/apps/:guid' do
11291129
let!(:process) { VCAP::CloudController::ProcessModelFactory.make(space: space) }
1130-
let(:k8s_api_client) { instance_double(Kubernetes::ApiClient, delete_image: nil) }
1130+
let(:k8s_api_client) { instance_double(Kubernetes::ApiClient, delete_image: nil, delete_custom_builder: nil) }
11311131

11321132
before do
11331133
allow(CloudController::DependencyLocator.instance).to receive(:k8s_api_client).and_return(k8s_api_client)

‎spec/unit/actions/app_delete_spec.rb‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ module VCAP::CloudController
1010

1111
let!(:app) { AppModel.make }
1212
let!(:app_dataset) { [app] }
13-
let(:k8s_api_client) { instance_double(Kubernetes::ApiClient, delete_image: nil) }
13+
let(:k8s_api_client) { instance_double(Kubernetes::ApiClient, delete_image: nil, delete_custom_builder: nil) }
1414
let(:route_resource_manager) { instance_double(Kubernetes::RouteResourceManager, update_destinations: nil) }
1515

1616
before do
@@ -48,9 +48,10 @@ module VCAP::CloudController
4848
app_delete.delete(app_dataset)
4949
end
5050

51-
it 'deletes the associated kpack Image' do
51+
it 'deletes the associated kpack resources' do
5252
build_namespace = VCAP::CloudController::Config.config.kpack_builder_namespace
5353
expect(k8s_api_client).to receive(:delete_image).with(app.guid, build_namespace)
54+
expect(k8s_api_client).to receive(:delete_custom_builder).with("app-#{app.guid}", build_namespace)
5455
app_delete.delete(app_dataset)
5556
end
5657

0 commit comments

Comments
 (0)