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

Commit 6b2e9b1

Browse files
cwlbraaSannidhi JalukarJaskanwal Pawar
committed
squash-merge kpack buildpack selection
- Build out KpackLifeCycle and associated validator - Fetch available buildpacks from k8s and validate availibility in k8s - using OpenStruct in test to emulate/statisfy duck-typing for the private KpackBuildpack class - constructs custombuilder from default custombuilder on create - once created, custombuilder updates don't work yet [#171029528] Co-authored-by: Sannidhi Jalukar <sjalukar@pivotal.io> Co-authored-by: Jaskanwal Pawar <jpawar@pivotal.io> Co-authored-by: Connor Braa <cbraa@pivotal.io>
1 parent c67edc7 commit 6b2e9b1

30 files changed

Lines changed: 617 additions & 70 deletions

app/actions/app_update.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ def custom_buildpacks_disabled?
5959
def validate_not_changing_lifecycle_type!(app, lifecycle)
6060
return if app.lifecycle_type == lifecycle.type
6161

62-
raise InvalidApp.new('Lifecycle type cannot be changed')
62+
raise InvalidApp.new("Lifecycle type cannot be changed from #{app.lifecycle_type} to #{lifecycle.type}")
6363
end
6464

6565
def existing_environment_variables_for(app)

app/messages/app_create_message.rb

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,15 +9,19 @@ def self.lifecycle_requested?
99
@lifecycle_requested ||= proc { |a| a.requested?(:lifecycle) }
1010
end
1111

12+
def self.lifecycle_type_requested?
13+
@lifecycle_type_requested ||= proc { |a| a.requested?(:lifecycle) && a.lifecycle_type.present? }
14+
end
15+
1216
validates_with NoAdditionalKeysValidator, RelationshipValidator
13-
validates_with LifecycleValidator, if: lifecycle_requested?
17+
validates_with LifecycleValidator, if: lifecycle_type_requested?
1418

1519
validates :name, string: true
1620
validates :environment_variables, hash: true, allow_nil: true
1721

1822
validates :lifecycle_type,
1923
string: true,
20-
if: lifecycle_requested?
24+
if: lifecycle_type_requested?
2125

2226
validates :lifecycle_data,
2327
hash: true,

app/messages/app_manifest_message.rb

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -260,7 +260,6 @@ def buildpacks_lifecycle_data
260260
end
261261

262262
{
263-
type: Lifecycles::BUILDPACK,
264263
data: {
265264
buildpacks: requested_buildpacks,
266265
stack: @stack

app/messages/app_update_message.rb

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,10 @@ def self.lifecycle_requested?
1111
@lifecycle_requested ||= proc { |a| a.requested?(:lifecycle) }
1212
end
1313

14+
def self.lifecycle_type_requested?
15+
@lifecycle_type_requested ||= proc { |a| a.requested?(:lifecycle) && a.lifecycle_type.present? }
16+
end
17+
1418
validates_with NoAdditionalKeysValidator
1519
validates_with LifecycleValidator, if: lifecycle_requested?
1620

@@ -19,7 +23,7 @@ def self.lifecycle_requested?
1923
validates :lifecycle_type,
2024
string: true,
2125
allow_nil: false,
22-
if: lifecycle_requested?
26+
if: lifecycle_type_requested?
2327

2428
validates :lifecycle_data,
2529
hash: true,

app/messages/validators.rb

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -160,10 +160,14 @@ def validate(record)
160160
data_message = {
161161
VCAP::CloudController::Lifecycles::BUILDPACK => VCAP::CloudController::BuildpackLifecycleDataMessage,
162162
VCAP::CloudController::Lifecycles::DOCKER => VCAP::CloudController::EmptyLifecycleDataMessage,
163-
VCAP::CloudController::Lifecycles::KPACK => VCAP::CloudController::EmptyLifecycleDataMessage,
163+
VCAP::CloudController::Lifecycles::KPACK => VCAP::CloudController::BuildpackLifecycleDataMessage,
164164
}
165165

166-
lifecycle_data_message_class = data_message[record.lifecycle_type]
166+
lifecycle_data_message_class = if record.lifecycle_type.present?
167+
data_message[record.lifecycle_type]
168+
else
169+
VCAP::CloudController::BuildpackLifecycleDataMessage
170+
end
167171
if lifecycle_data_message_class.nil?
168172
record.errors[:lifecycle_type].concat ["is not included in the list: #{data_message.keys.join(', ')}"]
169173
return

app/models/runtime/kpack_lifecycle_data_model.rb

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
module VCAP::CloudController
44
class KpackLifecycleDataModel < Sequel::Model(:kpack_lifecycle_data)
5+
include Serializer
6+
57
LIFECYCLE_TYPE = Lifecycles::KPACK
68

79
many_to_one :app,
@@ -22,6 +24,9 @@ class KpackLifecycleDataModel < Sequel::Model(:kpack_lifecycle_data)
2224
primary_key: :guid,
2325
without_guid_generation: true
2426

27+
# if this gets any thicker, we should model it properly in its own table
28+
serializes_via_json :buildpacks
29+
2530
def using_custom_buildpack?
2631
false
2732
end
@@ -34,11 +39,13 @@ def first_custom_buildpack_url
3439
return nil
3540
end
3641

37-
def buildpacks
38-
[]
39-
end
42+
# def buildpacks=(new_buildpacks) end
4043

41-
def buildpacks=(new_buildpacks) end
44+
def to_hash
45+
{
46+
buildpacks: buildpacks
47+
}
48+
end
4249

4350
def stack
4451
nil
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
require 'spec_helper'
2+
3+
module VCAP::CloudController
4+
RSpec.describe KpackLifecycleDataModel do
5+
subject(:lifecycle_data) { KpackLifecycleDataModel.new }
6+
7+
describe '#to_hash' do
8+
let(:expected_lifecycle_data) do
9+
{ buildpacks: buildpacks || [] }
10+
end
11+
let(:buildpacks) { [buildpack] }
12+
let(:buildpack) { 'ruby' }
13+
14+
before do
15+
Buildpack.make(name: 'ruby')
16+
lifecycle_data.buildpacks = buildpacks
17+
lifecycle_data.save
18+
end
19+
20+
it 'returns the lifecycle data as a hash' do
21+
expect(lifecycle_data.to_hash).to eq expected_lifecycle_data
22+
end
23+
24+
context 'when the user has not specified a buildpack' do
25+
let(:buildpacks) { [] }
26+
27+
it 'returns the lifecycle data as a hash' do
28+
expect(lifecycle_data.to_hash).to eq expected_lifecycle_data
29+
end
30+
end
31+
end
32+
end
33+
end
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
Sequel.migration do
2+
change do
3+
alter_table :kpack_lifecycle_data do
4+
add_column :buildpacks, String, size: 5000, default: MultiJson.dump([])
5+
end
6+
end
7+
end

lib/cloud_controller/diego/lifecycles/app_kpack_lifecycle.rb

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,24 @@
11
module VCAP::CloudController
22
class AppKpackLifecycle
3-
def initialize(*_message); end
3+
def initialize(message)
4+
@message = message
5+
end
46

57
def create_lifecycle_data_model(app)
68
app.kpack_lifecycle_data = KpackLifecycleDataModel.create(app: app)
79
end
810

9-
def update_lifecycle_data_model(_); end
11+
def update_lifecycle_data_model(app)
12+
if [update_lifecycle_data_buildpacks(app)].any?
13+
app.lifecycle_data.save
14+
end
15+
end
16+
17+
def update_lifecycle_data_buildpacks(app)
18+
if @message.buildpack_data.requested?(:buildpacks)
19+
app.lifecycle_data.update(buildpacks: @message.buildpack_data.buildpacks)
20+
end
21+
end
1022

1123
def valid?
1224
true

lib/cloud_controller/diego/lifecycles/app_lifecycle_provider.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ def self.provide_for_update(message, app)
2020
end
2121

2222
def self.provide(message, app)
23-
type = if message.requested?(:lifecycle)
23+
type = if message.lifecycle_type.present?
2424
message.lifecycle_type
2525
elsif !app.nil?
2626
app.lifecycle_type

0 commit comments

Comments
 (0)