Skip to content
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

Add HPA #305

Merged
merged 4 commits into from
Aug 7, 2018
Merged
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
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,12 @@

*Enhancements*
- Add Job resource class ([#295](https://github.com/Shopify/kubernetes-deploy/pull/296))
- Add CustomResourceDefinition resource class ([#305](https://github.com/Shopify/kubernetes-deploy/pull/306))
- Add CustomResourceDefinition resource class ([#306](https://github.com/Shopify/kubernetes-deploy/pull/306))
- Officially support Kubernetes 1.10 ([#308](https://github.com/Shopify/kubernetes-deploy/pull/308))
- SyncMediator will only batch fetch resources when there is a sufficiently large enough set of resources
being tracked ([#316](https://github.com/Shopify/kubernetes-deploy/pull/316))
- Allow CRs to be pruned based on `kubernetes-deploy.shopify.io/prunable` annotation on the custom resource definitions ([312](https://github.com/Shopify/kubernetes-deploy/pull/312))
- Add HorizontalPodAutoscaler resource class ([#305](https://github.com/Shopify/kubernetes-deploy/pull/305))

### 0.20.4
*Enhancements*
Expand Down
1 change: 1 addition & 0 deletions lib/kubernetes-deploy/deploy_task.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
cron_job
job
custom_resource_definition
horizontal_pod_autoscaler
).each do |subresource|
require "kubernetes-deploy/kubernetes_resource/#{subresource}"
end
Expand Down
8 changes: 8 additions & 0 deletions lib/kubernetes-deploy/kubeclient_builder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,14 @@ def build_apiextensions_v1beta1_kubeclient(context)
)
end

def build_autoscaling_v1_kubeclient(context)
_build_kubeclient(
api_version: "v2beta1",
context: context,
endpoint_path: "/apis/autoscaling"
)
end

def _build_kubeclient(api_version:, context:, endpoint_path: nil)
# Find a context defined in kube conf files that matches the input context by name
friendly_configs = config_files.map { |f| GoogleFriendlyConfig.read(f) }
Expand Down
6 changes: 5 additions & 1 deletion lib/kubernetes-deploy/kubernetes_resource.rb
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ def file_path
end

def sync(mediator)
@instance_data = mediator.get_instance(type, name)
@instance_data = mediator.get_instance(kubectl_resource_type, name)
end

def deploy_failed?
Expand Down Expand Up @@ -147,6 +147,10 @@ def type
@type || self.class.kind
end

def kubectl_resource_type
type
end

def deploy_timed_out?
return false unless deploy_started?
!deploy_succeeded? && !deploy_failed? && (Time.now.utc - @deploy_started_at > timeout)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
# frozen_string_literal: true
module KubernetesDeploy
class HorizontalPodAutoscaler < KubernetesResource
TIMEOUT = 3.minutes
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One test deploy took 120.5s , this should give us enough buffer

RECOVERABLE_CONDITIONS = %w(ScalingDisabled FailedGet)

def deploy_succeeded?
scaling_active_condition["status"] == "True"
end

def deploy_failed?
return false unless exists?
recoverable = RECOVERABLE_CONDITIONS.any? { |c| scaling_active_condition.fetch("reason", "").start_with?(c) }
scaling_active_condition["status"] == "False" && !recoverable
end

def kubectl_resource_type
'hpa.v2beta1.autoscaling'
end

def status
if !exists?
super
elsif deploy_succeeded?
"Configured"
elsif scaling_active_condition.present? || able_to_scale_condition.present?
condition = scaling_active_condition.presence || able_to_scale_condition
condition['reason']
else
"Unknown"
end
end

def failure_message
condition = scaling_active_condition.presence || able_to_scale_condition.presence || {}
condition['message']
end

def timeout_message
failure_message.presence || super
end

private

def conditions
@instance_data.dig("status", "conditions") || []
end

def able_to_scale_condition
conditions.detect { |c| c["type"] == "AbleToScale" } || {}
end

def scaling_active_condition
conditions.detect { |c| c["type"] == "ScalingActive" } || {}
end
end
end
2 changes: 1 addition & 1 deletion lib/kubernetes-deploy/sync_mediator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ def sync(resources)
dependencies = resources.map(&:class).uniq.flat_map do |c|
c::SYNC_DEPENDENCIES if c.const_defined?('SYNC_DEPENDENCIES')
end
kinds = (resources.map(&:type) + dependencies).compact.uniq
kinds = (resources.map(&:kubectl_resource_type) + dependencies).compact.uniq
kinds.each { |kind| fetch_by_kind(kind) }
end

Expand Down
21 changes: 21 additions & 0 deletions test/fixtures/hpa/deployment.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
apiVersion: apps/v1beta1
kind: Deployment
metadata:
name: hpa-deployment
spec:
replicas: 1
progressDeadlineSeconds: 60
template:
metadata:
labels:
name: hpa
app: hpa-deployment
spec:
containers:
- name: app
image: busybox
imagePullPolicy: IfNotPresent
command: ["tail", "-f", "/dev/null"]
resources:
requests:
cpu: 200m
16 changes: 16 additions & 0 deletions test/fixtures/hpa/hpa.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
apiVersion: autoscaling/v2beta1
kind: HorizontalPodAutoscaler
metadata:
name: hello-hpa
spec:
scaleTargetRef:
apiVersion: apps/v1
kind: Deployment
name: hpa-deployment
minReplicas: 1
maxReplicas: 2
metrics:
- type: Resource
resource:
name: cpu
targetAverageUtilization: 50
4 changes: 4 additions & 0 deletions test/helpers/kubeclient_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,4 +33,8 @@ def batch_v1_kubeclient
def apiextensions_v1beta1_kubeclient
@apiextensions_v1beta1_kubeclient ||= build_apiextensions_v1beta1_kubeclient(MINIKUBE_CONTEXT)
end

def autoscaling_v1_kubeclient
@autoscaling_v1_kubeclient ||= build_autoscaling_v1_kubeclient(MINIKUBE_CONTEXT)
end
end
17 changes: 17 additions & 0 deletions test/integration/kubernetes_deploy_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1053,4 +1053,21 @@ def test_raise_on_yaml_missing_kind
" datapoint: value1"
], in_order: true)
end

def test_hpa_can_be_successful
skip if KUBE_SERVER_VERSION < Gem::Version.new('1.9.0')
assert_deploy_success(deploy_fixtures("hpa"))
assert_logs_match_all([
"Deploying resources:",
"HorizontalPodAutoscaler/hello-hpa (timeout: 180s)",
%r{HorizontalPodAutoscaler/hello-hpa\s+Configured}
])
end

def test_hpa_can_be_pruned
skip if KUBE_SERVER_VERSION < Gem::Version.new('1.9.0')
assert_deploy_success(deploy_fixtures("hpa"))
assert_deploy_success(deploy_fixtures("hpa", subset: ["deployment.yml"]))
assert_logs_match_all([/The following resources were pruned: horizontalpodautoscaler(.autoscaling)? "hello-hpa"/])
end
end
13 changes: 13 additions & 0 deletions test/setup/metrics-server/auth-delegator.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
---
apiVersion: rbac.authorization.k8s.io/v1beta1
kind: ClusterRoleBinding
metadata:
name: metrics-server:system:auth-delegator
roleRef:
apiGroup: rbac.authorization.k8s.io
kind: ClusterRole
name: system:auth-delegator
subjects:
- kind: ServiceAccount
name: metrics-server
namespace: kube-system
14 changes: 14 additions & 0 deletions test/setup/metrics-server/auth-reader.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
---
apiVersion: rbac.authorization.k8s.io/v1beta1
kind: RoleBinding
metadata:
name: metrics-server-auth-reader
namespace: kube-system
roleRef:
apiGroup: rbac.authorization.k8s.io
kind: Role
name: extension-apiserver-authentication-reader
subjects:
- kind: ServiceAccount
name: metrics-server
namespace: kube-system
14 changes: 14 additions & 0 deletions test/setup/metrics-server/metrics-apiservice.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
---
apiVersion: apiregistration.k8s.io/v1beta1
kind: APIService
metadata:
name: v1beta1.metrics.k8s.io
spec:
service:
name: metrics-server
namespace: kube-system
group: metrics.k8s.io
version: v1beta1
insecureSkipTLSVerify: true
groupPriorityMinimum: 100
versionPriority: 100
32 changes: 32 additions & 0 deletions test/setup/metrics-server/metrics-server-deployment.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
---
apiVersion: v1
kind: ServiceAccount
metadata:
name: metrics-server
namespace: kube-system
---
apiVersion: extensions/v1beta1
kind: Deployment
metadata:
name: metrics-server
namespace: kube-system
labels:
k8s-app: metrics-server
spec:
selector:
matchLabels:
k8s-app: metrics-server
template:
metadata:
name: metrics-server
labels:
k8s-app: metrics-server
spec:
serviceAccountName: metrics-server
containers:
- name: metrics-server
image: gcr.io/google_containers/metrics-server-amd64:v0.2.1
imagePullPolicy: Always
command:
- /metrics-server
- --source=kubernetes.summary_api:''
15 changes: 15 additions & 0 deletions test/setup/metrics-server/metrics-server-service.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
---
apiVersion: v1
kind: Service
metadata:
name: metrics-server
namespace: kube-system
labels:
kubernetes.io/name: "Metrics-server"
spec:
selector:
k8s-app: metrics-server
ports:
- port: 443
protocol: TCP
targetPort: 443
38 changes: 38 additions & 0 deletions test/setup/metrics-server/resource-reader.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
---
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
name: system:metrics-server
rules:
- apiGroups:
- ""
resources:
- pods
- nodes
- nodes/stats
- namespaces
verbs:
- get
- list
- watch
- apiGroups:
- "extensions"
resources:
- deployments
verbs:
- get
- list
- watch
---
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRoleBinding
metadata:
name: system:metrics-server
roleRef:
apiGroup: rbac.authorization.k8s.io
kind: ClusterRole
name: system:metrics-server
subjects:
- kind: ServiceAccount
name: metrics-server
namespace: kube-system
13 changes: 13 additions & 0 deletions test/test_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -282,10 +282,23 @@ def self.prepare_pv(name)
}
kubeclient.create_persistent_volume(pv)
end

def self.deploy_metric_server
# Set-up the metric server that the HPA needs https://github.com/kubernetes-incubator/metrics-server
logger = KubernetesDeploy::FormattedLogger.build("default", KubeclientHelper::MINIKUBE_CONTEXT, $stderr)
kubectl = KubernetesDeploy::Kubectl.new(namespace: "kube-system", context: KubeclientHelper::MINIKUBE_CONTEXT,
logger: logger, log_failure_by_default: true, default_timeout: '5s')

Dir.glob("test/setup/metrics-server/*.{yml,yaml}*").map do |resource|
found = kubectl.run("get", "-f", resource, log_failure: false).last.success?
kubectl.run("create", "-f", resource) unless found
end
end
end

WebMock.allow_net_connect!
TestProvisioner.prepare_pv("pv0001")
TestProvisioner.prepare_pv("pv0002")
TestProvisioner.deploy_metric_server
WebMock.disable_net_connect!
end
16 changes: 16 additions & 0 deletions test/unit/sync_mediator_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,12 @@ def test_sync_does_not_warm_cache_with_few_resources
mediator.get_instance('FakeConfigMap', @fake_cm.name)
end

def test_sync_uses_kubectl_resource_type
hpa = FakeHPA.new('fake')
stub_kubectl_response('get', hpa.kubectl_resource_type, *@params, resp: { "items" => [] }, times: 1)
mediator.sync([hpa] * (KubernetesDeploy::SyncMediator::LARGE_BATCH_THRESHOLD + 1))
end

private

def mediator
Expand All @@ -164,6 +170,10 @@ def type
self.class.name.demodulize
end

def kubectl_resource_type
type
end

def kubectl_response
{
"metadata" => {
Expand All @@ -189,4 +199,10 @@ def sync(mediator)
mediator.sync([]) # clears the cache
end
end

class FakeHPA < MockResource
def kubectl_resource_type
'Not-HPA-Type'
end
end
end