Skip to content

[PoC] Implement RFC Improve Stack management #4475

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 32 additions & 5 deletions app/actions/build_create.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ def create_and_stage(package:, lifecycle:, metadata: nil, start_after_staging: f
raise InvalidPackage.new('Cannot stage package whose state is not ready.') if package.state != PackageModel::READY_STATE

requested_buildpacks_disabled!(lifecycle)
validate_stack!(lifecycle, package)

staging_details = get_staging_details(package, lifecycle)
staging_details.start_after_staging = start_after_staging
Expand Down Expand Up @@ -74,11 +75,13 @@ def create_and_stage(package:, lifecycle:, metadata: nil, start_after_staging: f

Repositories::AppUsageEventRepository.new.create_from_build(build, 'STAGING_STARTED')
app = package.app
Repositories::BuildEventRepository.record_build_create(build,
@user_audit_info,
app.name,
app.space_guid,
app.organization_guid)
Repositories::BuildEventRepository.record_build_create(
build,
@user_audit_info,
app.name,
app.space_guid,
app.organization_guid
)
end

logger.info("build created: #{build.guid}")
Expand All @@ -93,6 +96,30 @@ def create_and_stage(package:, lifecycle:, metadata: nil, start_after_staging: f

private

def validate_stack!(lifecycle, package)
return unless lifecycle.type == Lifecycles::BUILDPACK

stack = Stack.find(name: lifecycle.staging_stack)
return unless stack

if stack.disabled?
raise CloudController::Errors::ApiError.new_from_details(
'StackDisabled',
"Cannot stage app, stack '#{stack.name}' is disabled."
)
end

if stack.locked? && package.app.processes.empty?
raise CloudController::Errors::ApiError.new_from_details(
'StackLocked',
"Cannot stage new app, stack '#{stack.name}' is locked."
)
end

# This is a warning, so we just log it.
logger.warn("Stack '#{stack.name}' is deprecated.") if stack.deprecated?
end

def requested_buildpacks_disabled!(lifecycle)
return if lifecycle.type == Lifecycles::DOCKER

Expand Down
5 changes: 4 additions & 1 deletion app/actions/stack_create.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,10 @@ class Error < ::StandardError
def create(message)
stack = VCAP::CloudController::Stack.create(
name: message.name,
description: message.description
description: message.description,
deprecated_at: message.deprecated_at,
locked_at: message.locked_at,
disabled_at: message.disabled_at
)

MetadataUpdate.update(stack, message)
Expand Down
7 changes: 7 additions & 0 deletions app/actions/stack_update.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,13 @@ def initialize

def update(stack, message)
stack.db.transaction do
# Update stack attributes (excluding metadata which is handled separately)
stack_attributes = {}
%i[deprecated_at locked_at disabled_at].each do |attr|
stack_attributes[attr] = message.public_send(attr) if message.requested?(attr)
end
stack.set(stack_attributes) if stack_attributes.any?
stack.save
MetadataUpdate.update(stack, message)
end
@logger.info("Finished updating metadata on stack #{stack.guid}")
Expand Down
2 changes: 2 additions & 0 deletions app/controllers/v3/stacks_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@ def update
stack = StackUpdate.new.update(stack, message)

render status: :ok, json: Presenters::V3::StackPresenter.new(stack)
rescue StackUpdate::InvalidStack => e
unprocessable! e
end

def show_apps
Expand Down
5 changes: 4 additions & 1 deletion app/messages/stack_create_message.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,12 @@

module VCAP::CloudController
class StackCreateMessage < MetadataBaseMessage
register_allowed_keys %i[name description]
register_allowed_keys %i[name description deprecated_at locked_at disabled_at]

validates :name, presence: true, length: { maximum: 250 }
validates :description, length: { maximum: 250 }
validates :deprecated_at, simple_timestamp: true, allow_nil: true
validates :locked_at, simple_timestamp: true, allow_nil: true
validates :disabled_at, simple_timestamp: true, allow_nil: true
end
end
6 changes: 5 additions & 1 deletion app/messages/stack_update_message.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,12 @@

module VCAP::CloudController
class StackUpdateMessage < MetadataBaseMessage
register_allowed_keys []
register_allowed_keys %i[deprecated_at locked_at disabled_at]

validates_with NoAdditionalKeysValidator

validates :deprecated_at, simple_timestamp: true, allow_nil: true
validates :locked_at, simple_timestamp: true, allow_nil: true
validates :disabled_at, simple_timestamp: true, allow_nil: true
end
end
29 changes: 21 additions & 8 deletions app/messages/validators.rb
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,28 @@ def is_semver?(value)
end
end

module TimestampValidationHelper
private

def opinionated_iso_8601(timestamp, record, attribute)
return if timestamp.nil?
return unless /\A\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}Z\Z/ !~ timestamp.to_s

record.errors.add(attribute, message: "has an invalid timestamp format. Timestamps should be formatted as 'YYYY-MM-DDThh:mm:ssZ'")
end
end

class SimpleTimestampValidator < ActiveModel::EachValidator
include TimestampValidationHelper

def validate_each(record, attribute, value)
opinionated_iso_8601(value, record, attribute)
end
end

class TimestampValidator < ActiveModel::EachValidator
include TimestampValidationHelper

def validate_each(record, attribute, values)
if values.is_a?(Array)
values.each do |timestamp|
Expand Down Expand Up @@ -372,14 +393,6 @@ def validate_each(record, attribute, values)
end
end
end

private

def opinionated_iso_8601(timestamp, record, attribute)
return unless /\A\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}Z\Z/ !~ timestamp.to_s

record.errors.add(attribute, message: "has an invalid timestamp format. Timestamps should be formatted as 'YYYY-MM-DDThh:mm:ssZ'")
end
end

class TargetGuidsValidator < ActiveModel::Validator
Expand Down
31 changes: 28 additions & 3 deletions app/models/runtime/stack.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ class AppsStillPresentError < StandardError

plugin :serialization

export_attributes :name, :description, :build_rootfs_image, :run_rootfs_image
import_attributes :name, :description, :build_rootfs_image, :run_rootfs_image
export_attributes :name, :description, :build_rootfs_image, :run_rootfs_image, :deprecated_at, :locked_at, :disabled_at
import_attributes :name, :description, :build_rootfs_image, :run_rootfs_image, :deprecated_at, :locked_at, :disabled_at

strip_attributes :name

Expand All @@ -43,6 +43,19 @@ def around_save
def validate
validates_presence :name
validates_unique :name
validate_timestamp_ordering
end

def deprecated?
deprecated_at && deprecated_at <= Time.now
end

def locked?
locked_at && locked_at <= Time.now
end

def disabled?
disabled_at && disabled_at <= Time.now
end

def before_destroy
Expand Down Expand Up @@ -95,8 +108,20 @@ def self.populate_from_hash(hash)
stack.set(hash)
Steno.logger('cc.stack').warn('stack.populate.collision', hash) if stack.modified?
else
create(hash.slice('name', 'description', 'build_rootfs_image', 'run_rootfs_image'))
create(hash.slice('name', 'description', 'build_rootfs_image', 'run_rootfs_image', 'deprecated_at', 'locked_at', 'disabled_at'))
end
end

private

def validate_timestamp_ordering
return unless [deprecated_at, locked_at, disabled_at].any?

errors.add(:deprecated_at, 'must be before locked_at') if deprecated_at && locked_at && deprecated_at > locked_at

return unless locked_at && disabled_at && locked_at > disabled_at

errors.add(:locked_at, 'must be before disabled_at')
end
end
end
3 changes: 3 additions & 0 deletions app/presenters/v3/stack_presenter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ def to_hash
run_rootfs_image: stack.run_rootfs_image,
build_rootfs_image: stack.build_rootfs_image,
default: stack.default?,
deprecated_at: stack.deprecated_at,
locked_at: stack.locked_at,
disabled_at: stack.disabled_at,
metadata: {
labels: hashified_labels(stack.labels),
annotations: hashified_annotations(stack.annotations)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
Sequel.migration do
change do
alter_table(:stacks) do
add_column :deprecated_at, DateTime, null: true
add_column :locked_at, DateTime, null: true
add_column :disabled_at, DateTime, null: true
end
end
end
10 changes: 10 additions & 0 deletions errors/v2.yml
Original file line number Diff line number Diff line change
Expand Up @@ -863,6 +863,16 @@
http_code: 404
message: "The stack could not be found: %s"

250004:
name: StackDisabled
http_code: 422
message: "Cannot stage app, stack '%s' is disabled."

250005:
name: StackLocked
http_code: 422
message: "Cannot stage new app, stack '%s' is locked."

260001:
name: ServicePlanVisibilityInvalid
http_code: 400
Expand Down
21 changes: 19 additions & 2 deletions lib/cloud_controller/diego/buildpack/staging_action_builder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,33 @@ def task_environment_variables

private

def lifecycle
staging_details.lifecycle
end

def stage_action
staging_details_env = BbsEnvironmentBuilder.build(staging_details.environment_variables)

# Stack deprecation warning will be handled by the build_create action
buildpack_keys = if lifecycle.respond_to?(:buildpack_infos)
lifecycle.buildpack_infos.map(&:key)
else
lifecycle_data[:buildpacks]&.map { |bp| bp[:key] } || [] # rubocop:disable Rails/Pluck
end

skip_detect = if lifecycle.respond_to?(:skip_detect?)
lifecycle.skip_detect?
else
lifecycle_data[:buildpacks]&.any? { |bp| bp[:skip_detect] } || false
end

::Diego::Bbs::Models::RunAction.new(
path: '/tmp/lifecycle/builder',
user: 'vcap',
args: [
"-buildpackOrder=#{lifecycle_data[:buildpacks].pluck(:key).join(',')}",
"-buildpackOrder=#{buildpack_keys.join(',')}",
"-skipCertVerify=#{config.get(:skip_cert_verify)}",
"-skipDetect=#{skip_detect?}",
"-skipDetect=#{skip_detect}",
'-buildDir=/tmp/app',
'-outputDroplet=/tmp/droplet',
'-outputMetadata=/tmp/result.json',
Expand Down
Loading