From 52561bd15d9bb7a2e92847fad09639ec47ed43d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=E1=B4=87=CA=80=C9=B4=E1=B4=85=20S=E1=B4=84=CA=9C?= =?UTF-8?q?=E1=B4=8F=CA=80=C9=A2=E1=B4=87=CA=80s?= <6213398+bjw-s@users.noreply.github.com> Date: Tue, 5 Jan 2021 18:34:42 +0100 Subject: [PATCH] [common] version 2.2.0 (#443) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Add code-server env to values * Bump chart version * [common] - add ingressclass name (#441) * add ingressclassname Signed-off-by: Jon Baker * Allow disabling of service links * Update changelog * Update changelog * Set better default for targetPort * Allow for different container / service ports * Fix linting error * Add unit tests * Add unit tests * Add unit tests * Service unit tests * Split CI jobs * Testing CI * Testing CI * Testing CI * Testing CI * Rename test file to match source * Fix newline Co-authored-by: Jon Baker Co-authored-by: ᗪєνιη ᗷυнʟ --- .github/workflows/lint-test.yaml | 63 +++++++++- .gitignore | 10 +- Gemfile | 12 ++ charts/common/CHANGELOG.md | 21 ++++ charts/common/Chart.yaml | 2 +- charts/common/templates/classes/_ingress.tpl | 5 + .../templates/classes/_service_ports.tpl | 2 +- .../common/templates/lib/controller/_pod.tpl | 1 + .../templates/lib/controller/_ports.tpl | 2 +- charts/common/values.yaml | 16 ++- test/charts/common-test_spec.rb | 86 +++++++++++++ test/test_helper.rb | 119 ++++++++++++++++++ 12 files changed, 328 insertions(+), 11 deletions(-) create mode 100644 Gemfile create mode 100644 test/charts/common-test_spec.rb create mode 100644 test/test_helper.rb diff --git a/.github/workflows/lint-test.yaml b/.github/workflows/lint-test.yaml index 8f6f21b8..bc3c6e8d 100644 --- a/.github/workflows/lint-test.yaml +++ b/.github/workflows/lint-test.yaml @@ -3,8 +3,11 @@ name: Lint and Test Charts on: pull_request jobs: - lint-test: + lint: runs-on: ubuntu-latest + outputs: + changed: ${{ steps.list-changed.outputs.changed }} + common: ${{ steps.list-changed.outputs.common }} steps: - name: Checkout uses: actions/checkout@v2 @@ -41,15 +44,67 @@ jobs: run: ct lint --config .github/ct.yaml --excluded-charts "" if: steps.list-changed.outputs.changed == 'true' || steps.list-changed.outputs.common == 'true' + unittest: + runs-on: ubuntu-latest + needs: lint + steps: + - name: Checkout + uses: actions/checkout@v2 + with: + fetch-depth: 0 + + - name: Install Dev tools + run: sudo apt-get update && sudo apt-get install -y jq libjq-dev + + - name: Install Helm + uses: azure/setup-helm@v1 + with: + version: v3.4.0 + + - name: Install Ruby + uses: ruby/setup-ruby@v1 + with: + ruby-version: 2.7 + + - name: Install dependencies + run: | + export RUBYJQ_USE_SYSTEM_LIBRARIES=1 + bundle install + + - name: Run tests + run: | + bundle exec m -r test/charts + + install: + runs-on: ubuntu-latest + needs: lint + steps: + - name: Checkout + uses: actions/checkout@v2 + with: + fetch-depth: 0 + + - name: Install Helm + uses: azure/setup-helm@v1 + with: + version: v3.4.0 + + - uses: actions/setup-python@v2 + with: + python-version: 3.7 + + - name: Set up chart-testing + uses: helm/chart-testing-action@v2.0.1 + - name: Create kind cluster uses: helm/kind-action@v1.1.0 - if: steps.list-changed.outputs.changed == 'true' || steps.list-changed.outputs.common == 'true' + if: needs.lint.outputs.changed == 'true' || needs.lint.outputs.common == 'true' - name: Run chart-testing (install) run: ct install --config .github/ct.yaml - if: steps.list-changed.outputs.changed == 'true' + if: needs.lint.outputs.changed == 'true' - name: Run chart-testing (common-test) run: | ct install --config .github/ct.yaml --charts 'charts/common-test' - if: steps.list-changed.outputs.common == 'true' + if: needs.lint.outputs.common == 'true' diff --git a/.gitignore b/.gitignore index 1520b7ff..8b579320 100644 --- a/.gitignore +++ b/.gitignore @@ -1,4 +1,12 @@ -.env +# IDE resources +.vscode .idea +.devcontainer/ + +# Helm resources charts/*/Chart.lock charts/*/charts + +# Other rsources +.env +Gemfile.lock diff --git a/Gemfile b/Gemfile new file mode 100644 index 00000000..ac5b8e73 --- /dev/null +++ b/Gemfile @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +source 'https://rubygems.org' + +group :test do + gem 'm' + gem 'minitest' + gem 'minitest-implicit-subject' + gem 'minitest-reporters' + gem 'pry' + gem 'ruby-jq' +end diff --git a/charts/common/CHANGELOG.md b/charts/common/CHANGELOG.md index 566628a2..12d0116b 100644 --- a/charts/common/CHANGELOG.md +++ b/charts/common/CHANGELOG.md @@ -4,6 +4,23 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [2.2.0] + +### Added + +- Allow serviceLinks to be enabled/disabled. +- Support for ingressClassName on apiVersion `networking.k8s.io/v1` by setting `ingress.ingressClassName`. +- Added some initial proper unit tests for the `common` chart. + +### Changed + +- `service.port.targetPort` is now used in the container spec instead of `service.port.port` if specified. This allows for different service and container ports. (Implements [#465](https://github.com/k8s-at-home/charts/issues/465)). + +### Fixed + +- Document setting environment variables for code-server add-on in `values.yaml` (Fixes [#436](https://github.com/k8s-at-home/charts/issues/436)). +- Set service targetPort to the service port name first if no `targetPort` value is given. + ## [2.1.0] ### Added @@ -36,6 +53,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 This is the last version before starting this changelog. All sorts of cool stuff was changed, but only `git log` remembers what that was :slightly_frowning_face: +[2.2.0]: https://github.com/k8s-at-home/charts/tree/common-2.2.0/charts/common + +[2.1.0]: https://github.com/k8s-at-home/charts/tree/common-2.1.0/charts/common + [2.0.4]: https://github.com/k8s-at-home/charts/tree/common-2.0.4/charts/common [2.0.0]: https://github.com/k8s-at-home/charts/tree/common-2.0.0/charts/common diff --git a/charts/common/Chart.yaml b/charts/common/Chart.yaml index 199b4766..29362361 100644 --- a/charts/common/Chart.yaml +++ b/charts/common/Chart.yaml @@ -2,7 +2,7 @@ apiVersion: v2 name: common description: Function library for k8s-at-home charts type: library -version: 2.1.1 +version: 2.2.0 keywords: - k8s-at-home - common diff --git a/charts/common/templates/classes/_ingress.tpl b/charts/common/templates/classes/_ingress.tpl index ab9774d5..713d3a3b 100644 --- a/charts/common/templates/classes/_ingress.tpl +++ b/charts/common/templates/classes/_ingress.tpl @@ -26,6 +26,11 @@ metadata: {{- toYaml . | nindent 4 }} {{- end }} spec: + {{- if eq (include "common.capabilities.ingress.apiVersion" $) "networking.k8s.io/v1" }} + {{- if $values.ingressClassName }} + ingressClassName: {{ $values.ingressClassName }} + {{- end }} + {{- end }} {{- if $values.tls }} tls: {{- range $values.tls }} diff --git a/charts/common/templates/classes/_service_ports.tpl b/charts/common/templates/classes/_service_ports.tpl index 2c8f56d2..14832e6f 100644 --- a/charts/common/templates/classes/_service_ports.tpl +++ b/charts/common/templates/classes/_service_ports.tpl @@ -12,7 +12,7 @@ Render all the ports and additionalPorts for a Service object. ports: {{- range $_ := $ports }} - port: {{ .port }} - targetPort: {{ .targetPort | default "http" }} + targetPort: {{ .targetPort | default .name | default "http" }} protocol: {{ .protocol | default "TCP" }} name: {{ .name | default "http" }} {{- if (and (eq $.svcType "NodePort") (not (empty .nodePort))) }} diff --git a/charts/common/templates/lib/controller/_pod.tpl b/charts/common/templates/lib/controller/_pod.tpl index 1cb3701e..8743191a 100644 --- a/charts/common/templates/lib/controller/_pod.tpl +++ b/charts/common/templates/lib/controller/_pod.tpl @@ -21,6 +21,7 @@ dnsPolicy: {{ . }} dnsConfig: {{- toYaml . | nindent 2 }} {{- end }} +enableServiceLinks: {{ .Values.enableServiceLinks }} {{- with .Values.initContainers }} initContainers: {{- toYaml . | nindent 2 }} diff --git a/charts/common/templates/lib/controller/_ports.tpl b/charts/common/templates/lib/controller/_ports.tpl index 020a85cf..c7fc4ac6 100644 --- a/charts/common/templates/lib/controller/_ports.tpl +++ b/charts/common/templates/lib/controller/_ports.tpl @@ -32,7 +32,7 @@ Ports included by the controller. ports: {{- range $_ := $ports }} - name: {{ .name }} - containerPort: {{ .port }} + containerPort: {{ .targetPort | default .port }} protocol: {{ .protocol | default "TCP" }} {{- end -}} {{- end -}} diff --git a/charts/common/values.yaml b/charts/common/values.yaml index a8914d23..8c453b31 100644 --- a/charts/common/values.yaml +++ b/charts/common/values.yaml @@ -51,6 +51,11 @@ dnsPolicy: ClusterFirst # - name: ndots # value: "1" +# Enable/disable the generation of environment variables for services. +# See https://kubernetes.io/docs/concepts/services-networking/connect-applications-service/#accessing-the-service +# for more information. +enableServiceLinks: true + initContainers: [] additionalContainers: [] @@ -96,13 +101,14 @@ probes: service: enabled: true type: ClusterIP - # Specify the default port information + ## Specify the default port information port: port: - # name defaults to http + ## name defaults to http name: protocol: TCP - # targetPort defaults to http + ## targetPort defaults to the service name. If targetPort is specified, this port number + ## is used in the container definition instead of service.port.port. targetPort: ## Specify the nodePort value for the LoadBalancer and NodePort service types. ## ref: https://kubernetes.io/docs/concepts/services-networking/service/#type-nodeport @@ -314,6 +320,10 @@ addons: pullPolicy: IfNotPresent tag: 3.7.4 + # Set any environment variables for code-server here + env: {} + # TZ: UTC + # Set codeserver command line arguments # consider setting --user-data-dir to a persistent location to preserve code-server setting changes args: diff --git a/test/charts/common-test_spec.rb b/test/charts/common-test_spec.rb new file mode 100644 index 00000000..5fb270f5 --- /dev/null +++ b/test/charts/common-test_spec.rb @@ -0,0 +1,86 @@ +# frozen_string_literal: true +require_relative '../test_helper' + +class Test < ChartTest + @@chart = Chart.new('charts/common-test') + + describe @@chart.name do + describe 'controller type' do + it 'defaults to "Deployment"' do + assert_nil(resource('StatefulSet')) + assert_nil(resource('DaemonSet')) + refute_nil(resource('Deployment')) + end + + it 'accepts "statefulset"' do + chart.value controllerType: 'statefulset' + assert_nil(resource('Deployment')) + assert_nil(resource('DaemonSet')) + refute_nil(resource('StatefulSet')) + end + + it 'accepts "daemonset"' do + chart.value controllerType: 'daemonset' + assert_nil(resource('Deployment')) + assert_nil(resource('StatefulSet')) + refute_nil(resource('DaemonSet')) + end + end + + describe 'pod replicas' do + it 'defaults to 1' do + jq('.spec.replicas', resource('Deployment')).must_equal 1 + end + + it 'accepts integer as value' do + chart.value replicas: 3 + jq('.spec.replicas', resource('Deployment')).must_equal 3 + end + end + + describe 'ports settings' do + default_name = 'http' + default_port = 8080 + + it 'defaults to name "http" on port 8080' do + jq('.spec.ports[0].port', resource('Service')).must_equal default_port + jq('.spec.ports[0].targetPort', resource('Service')).must_equal default_name + jq('.spec.ports[0].name', resource('Service')).must_equal default_name + jq('.spec.template.spec.containers[0].ports[0].containerPort', resource('Deployment')).must_equal default_port + jq('.spec.template.spec.containers[0].ports[0].name', resource('Deployment')).must_equal default_name + end + + it 'port name can be overridden' do + values = { + service: { + port: { + name: 'server' + } + } + } + chart.value values + jq('.spec.ports[0].port', resource('Service')).must_equal default_port + jq('.spec.ports[0].targetPort', resource('Service')).must_equal values[:service][:port][:name] + jq('.spec.ports[0].name', resource('Service')).must_equal values[:service][:port][:name] + jq('.spec.template.spec.containers[0].ports[0].containerPort', resource('Deployment')).must_equal default_port + jq('.spec.template.spec.containers[0].ports[0].name', resource('Deployment')).must_equal values[:service][:port][:name] + end + + it 'targetPort can be overridden' do + values = { + service: { + port: { + targetPort: 80 + } + } + } + chart.value values + jq('.spec.ports[0].port', resource('Service')).must_equal default_port + jq('.spec.ports[0].targetPort', resource('Service')).must_equal values[:service][:port][:targetPort] + jq('.spec.ports[0].name', resource('Service')).must_equal default_name + jq('.spec.template.spec.containers[0].ports[0].containerPort', resource('Deployment')).must_equal values[:service][:port][:targetPort] + jq('.spec.template.spec.containers[0].ports[0].name', resource('Deployment')).must_equal default_name + end + end + end +end diff --git a/test/test_helper.rb b/test/test_helper.rb new file mode 100644 index 00000000..2deecf2f --- /dev/null +++ b/test/test_helper.rb @@ -0,0 +1,119 @@ +# frozen_string_literal: true + +require 'json' +require 'yaml' +require 'open3' + +require 'jq/extend' +require 'minitest-implicit-subject' +require "minitest/reporters" +require 'minitest/autorun' +require 'minitest/pride' + +class HelmCompileError < StandardError +end + +class HelmDepsError < StandardError +end + +class Chart + attr_reader :name, :path, :values + + def initialize(chart) + @name = chart.split('/').last + + @path = File.expand_path(chart) + + @values = default_values + + update_deps! + end + + def update_deps! + command = "helm dep update '#{path}'" + stdout, stderr, status = Open3.capture3(command) + raise HelmDepsError, stderr if status != 0 + end + + def reset! + @values = default_values + @parsed_resources = nil + end + + def value(value) + values.merge!(value) + end + + def configure_custom_name(name) + @name = name + end + + def execute_helm_template! + file = Tempfile.new(name) + file.write(JSON.parse(values.to_json).to_yaml) + file.close + + begin + command = "helm template '#{name}' '#{path}' --namespace='default' --values='#{file.path}'" + stdout, stderr, status = Open3.capture3(command) + + raise HelmCompileError, stderr if status != 0 + + stdout + ensure + file.unlink + end + end + + def parsed_resources + @parsed_resources ||= begin + output = execute_helm_template! + puts output if ENV.fetch('DEBUG', 'false') == 'true' + YAML.load_stream(output) + end + end + + def resources(matcher = nil) + return parsed_resources unless matcher + + parsed_resources.select do |r| + r >= Hash[matcher.map { |k, v| [k.to_s, v] }] + end + end + + def default_values + { + } + end +end + +class ExtendedMinitest < Minitest::Test + extend MiniTest::Spec::DSL +end + +class ChartTest < ExtendedMinitest + Minitest::Reporters.use! Minitest::Reporters::SpecReporter.new + + before do + chart.reset! + end + + def chart + self.class.class_variable_get('@@chart') + end + + def resource(name) + chart.resources(kind: name).first + end + + def jq(matcher, object) + value(object.jq(matcher)[0]) + end +end + +class Minitest::Result + def name + test_name = defined?(@name) ? @name : super + test_name.to_s.gsub /\Atest_\d{4,}_/, "" + end +end