Skip to content

Commit

Permalink
ci: add lint ci for helm chart.
Browse files Browse the repository at this point in the history
Signed-off-by: Lan Liang <gcslyp@gmail.com>
  • Loading branch information
liangyuanpeng committed Aug 8, 2024
1 parent a39f99b commit c52c7c1
Show file tree
Hide file tree
Showing 15 changed files with 130 additions and 16 deletions.
81 changes: 81 additions & 0 deletions .github/workflows/lint-chart.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
# validate any chart changes under charts directory
name: Chart Lint

env:
HELM_VERSION: v3.11.2
KIND_VERSION: v0.23.0
KIND_NODE_IMAGE: kindest/node:v1.30.0
K8S_VERSION: v1.30.0

on:
push:
# Exclude branches created by Dependabot to avoid triggering current workflow
# for PRs initiated by Dependabot.
branches-ignore:
- 'dependabot/**'
paths:
- "kwok/charts/**"
pull_request:
paths:
- "charts/**"

permissions:
contents: read

jobs:
chart-lint-test:
runs-on: ubuntu-22.04
steps:
- name: Checkout
uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332
with:
fetch-depth: 0

- name: Set up Helm
uses: azure/setup-helm@fe7b79cd5ee1e45176fcad797de68ecaf3ca4814 # v4.2.0
with:
version: ${{ env.HELM_VERSION }}

- name: Run chart-testing (template)
run: |
helm template --dependency-update ./kwok/charts --debug > /dev/null
# Python is required because `ct lint` runs Yamale (https://github.com/23andMe/Yamale) and
# yamllint (https://github.com/adrienverge/yamllint) which require Python
- name: Set up Python
uses: actions/setup-python@39cd14951b08e74b54015e9e001cdefcf80e669f # v5.1
with:
python-version: 3.10
check-latest: true

- name: Set up chart-testing
uses: helm/chart-testing-action@e6669bcd63d7cb57cb4380c33043eebe5d111992

- name: Add dependency chart repos
run: |
helm repo add bitnami https://charts.bitnami.com/bitnami
- name: Run chart-testing (list-changed)
id: list-changed
run: |

Check failure on line 60 in .github/workflows/lint-chart.yaml

View workflow job for this annotation

GitHub Actions / presubmit (1.25.x)

shellcheck reported issue in this script: SC2086:info:3:26: Double quote to prevent globbing and word splitting

Check failure on line 60 in .github/workflows/lint-chart.yaml

View workflow job for this annotation

GitHub Actions / presubmit (1.26.x)

shellcheck reported issue in this script: SC2086:info:3:26: Double quote to prevent globbing and word splitting

Check failure on line 60 in .github/workflows/lint-chart.yaml

View workflow job for this annotation

GitHub Actions / presubmit (1.27.x)

shellcheck reported issue in this script: SC2086:info:3:26: Double quote to prevent globbing and word splitting

Check failure on line 60 in .github/workflows/lint-chart.yaml

View workflow job for this annotation

GitHub Actions / presubmit (1.28.x)

shellcheck reported issue in this script: SC2086:info:3:26: Double quote to prevent globbing and word splitting

Check failure on line 60 in .github/workflows/lint-chart.yaml

View workflow job for this annotation

GitHub Actions / presubmit (1.29.x)

shellcheck reported issue in this script: SC2086:info:3:26: Double quote to prevent globbing and word splitting

Check failure on line 60 in .github/workflows/lint-chart.yaml

View workflow job for this annotation

GitHub Actions / presubmit (1.30.x)

shellcheck reported issue in this script: SC2086:info:3:26: Double quote to prevent globbing and word splitting
changed=$( ct list-changed )
if [[ -n "$changed" ]]; then
echo "changed=true" >> $GITHUB_OUTPUT
fi
- name: Run chart-testing (lint)
if: steps.list-changed.outputs.changed == 'true'
run: ct lint --debug --check-version-increment=false

- name: Create kind cluster
uses: helm/kind-action@0025e74a8c7512023d06dc019c617aa3cf561fde # v1.10.0
if: steps.list-changed.outputs.changed == 'true'
with:
wait: 120s
version: ${{ env.KIND_VERSION }}
node_image: ${{ env.KIND_NODE_IMAGE }}
kubectl_version: ${{ env.K8S_VERSION }}

- name: Run chart-testing (install)
if: steps.list-changed.outputs.changed == 'true'
run: ct install --debug --helm-extra-args "--timeout 800s"
1 change: 1 addition & 0 deletions kwok/charts/crds/karpenter.sh_nodeclaims.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ spec:
properties:
group:
description: API version of the referent
pattern: ^[^/]*$
type: string
kind:
description: 'Kind of the referent; More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds"'
Expand Down
1 change: 1 addition & 0 deletions kwok/charts/crds/karpenter.sh_nodepools.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,7 @@ spec:
properties:
group:
description: API version of the referent
pattern: ^[^/]*$
type: string
kind:
description: 'Kind of the referent; More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds"'
Expand Down
2 changes: 1 addition & 1 deletion kwok/charts/templates/role.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -75,4 +75,4 @@ rules:
# Write
- apiGroups: ["coordination.k8s.io"]
resources: ["leases"]
verbs: ["delete"]
verbs: ["delete"]
2 changes: 1 addition & 1 deletion kwok/charts/templates/secret-webhook-cert.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,4 @@ metadata:
{{- toYaml . | nindent 4 }}
{{- end }}
# data: {} # Injected by karpenter-webhook
{{- end }}
{{- end }}
1 change: 1 addition & 0 deletions pkg/apis/crds/karpenter.sh_nodeclaims.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ spec:
properties:
group:
description: API version of the referent
pattern: ^[^/]*$
type: string
kind:
description: 'Kind of the referent; More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds"'
Expand Down
1 change: 1 addition & 0 deletions pkg/apis/crds/karpenter.sh_nodepools.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,7 @@ spec:
properties:
group:
description: API version of the referent
pattern: ^[^/]*$
type: string
kind:
description: 'Kind of the referent; More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds"'
Expand Down
1 change: 1 addition & 0 deletions pkg/apis/v1/nodeclaim.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ type NodeClassReference struct {
// +required
Name string `json:"name"`
// API version of the referent
// +kubebuilder:validation:Pattern=`^[^/]*$`
// +required
Group string `json:"group"`
}
Expand Down
22 changes: 22 additions & 0 deletions pkg/apis/v1/nodeclaim_validation_cel_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@ import (
"strings"
"time"

"github.com/awslabs/operatorpkg/object"

"sigs.k8s.io/karpenter/pkg/test/v1alpha1"

"sigs.k8s.io/karpenter/pkg/test"

"github.com/Pallinder/go-randomdata"
Expand Down Expand Up @@ -95,6 +99,24 @@ var _ = Describe("Validation", func() {
Expect(env.Client.Create(ctx, nodeClaim)).To(Succeed())
})
})
Context("NodeClassRef", func() {
It("should succeed for valid group", func() {
nodeClaim.Spec.NodeClassRef = &NodeClassReference{
Kind: object.GVK(&v1alpha1.TestNodeClass{}).Kind,
Name: "nodeclass-test",
Group: object.GVK(&v1alpha1.TestNodeClass{}).Group,
}
Expect(env.Client.Create(ctx, nodeClaim)).To(Succeed())
})
It("should fail for invalid group", func() {
nodeClaim.Spec.NodeClassRef = &NodeClassReference{
Kind: object.GVK(&v1alpha1.TestNodeClass{}).Kind,
Name: "nodeclass-test",
Group: "karpenter.test.sh/v1",
}
Expect(env.Client.Create(ctx, nodeClaim)).ToNot(Succeed())
})
})
Context("Requirements", func() {
It("should allow supported ops", func() {
nodeClaim.Spec.Requirements = []NodeSelectorRequirementWithMinValues{
Expand Down
2 changes: 1 addition & 1 deletion pkg/controllers/nodeclaim/disruption/drift_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,7 @@ var _ = Describe("Drift", func() {
NodeClassRef: &v1.NodeClassReference{
Kind: "fakeKind",
Name: "fakeName",
Group: "fakeGroup/fakeVerion",
Group: "fakeGroup",
},
Taints: []corev1.Taint{
{
Expand Down
1 change: 1 addition & 0 deletions pkg/controllers/nodeclaim/lifecycle/registration.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ func (r *Registration) Reconcile(ctx context.Context, nodeClaim *v1.NodeClaim) (
// check if sync succeeded but setting the registered status condition failed
// if sync succeeded, then the label will be present and the taint will be gone
if _, ok := node.Labels[v1.NodeRegisteredLabelKey]; !ok && !hasStartupTaint {
nodeClaim.StatusConditions().SetFalse(v1.ConditionTypeRegistered, "UnregisteredTaintNotFound", fmt.Sprintf("Invariant violated, %s taint must be present on Karpenter-managed nodes", v1.UnregisteredTaintKey))
return reconcile.Result{}, fmt.Errorf("missing required startup taint, %s", v1.UnregisteredTaintKey)
}
ctx = log.IntoContext(ctx, log.FromContext(ctx).WithValues("Node", klog.KRef("", node.Name)))
Expand Down
2 changes: 2 additions & 0 deletions pkg/controllers/nodeclaim/lifecycle/registration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,8 @@ var _ = Describe("Registration", func() {
node := test.Node(test.NodeOptions{ProviderID: nodeClaim.Status.ProviderID})
ExpectApplied(ctx, env.Client, node)
_ = ExpectObjectReconcileFailed(ctx, env.Client, nodeClaimController, nodeClaim)
nodeClaim = ExpectExists(ctx, env.Client, nodeClaim)
Expect(ExpectStatusConditionExists(nodeClaim, v1.ConditionTypeRegistered).Status).To(Equal(metav1.ConditionFalse))
})
It("should sync the labels to the Node when the Node comes online", func() {
nodeClaim := test.NodeClaim(v1.NodeClaim{
Expand Down
4 changes: 2 additions & 2 deletions pkg/controllers/provisioning/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1260,7 +1260,7 @@ var _ = Describe("Provisioning", func() {
Template: v1.NodeClaimTemplate{
Spec: v1.NodeClaimTemplateSpec{
NodeClassRef: &v1.NodeClassReference{
Group: "cloudprovider.karpenter.sh/v1",
Group: "cloudprovider.karpenter.sh",
Kind: "CloudProvider",
Name: "default",
},
Expand All @@ -1275,7 +1275,7 @@ var _ = Describe("Provisioning", func() {
Expect(cloudProvider.CreateCalls).To(HaveLen(1))
Expect(cloudProvider.CreateCalls[0].Spec.NodeClassRef).To(Equal(
&v1.NodeClassReference{
Group: "cloudprovider.karpenter.sh/v1",
Group: "cloudprovider.karpenter.sh",
Kind: "CloudProvider",
Name: "default",
},
Expand Down
23 changes: 13 additions & 10 deletions pkg/scheduling/taints.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,24 +21,27 @@ import (

"github.com/samber/lo"
"go.uber.org/multierr"
v1 "k8s.io/api/core/v1"
corev1 "k8s.io/api/core/v1"
cloudproviderapi "k8s.io/cloud-provider/api"

v1 "sigs.k8s.io/karpenter/pkg/apis/v1"
)

// KnownEphemeralTaints are taints that are expected to be added to a node while it's initializing
// If the node is a Karpenter-managed node, we don't consider these taints while the node is uninitialized
// since we expect these taints to eventually be removed
var KnownEphemeralTaints = []v1.Taint{
{Key: v1.TaintNodeNotReady, Effect: v1.TaintEffectNoSchedule},
{Key: v1.TaintNodeUnreachable, Effect: v1.TaintEffectNoSchedule},
{Key: cloudproviderapi.TaintExternalCloudProvider, Effect: v1.TaintEffectNoSchedule, Value: "true"},
var KnownEphemeralTaints = []corev1.Taint{
{Key: corev1.TaintNodeNotReady, Effect: corev1.TaintEffectNoSchedule},
{Key: corev1.TaintNodeUnreachable, Effect: corev1.TaintEffectNoSchedule},
{Key: cloudproviderapi.TaintExternalCloudProvider, Effect: corev1.TaintEffectNoSchedule, Value: "true"},
v1.UnregisteredNoExecuteTaint,
}

// Taints is a decorated alias type for []v1.Taint
type Taints []v1.Taint
// Taints is a decorated alias type for []corev1.Taint
type Taints []corev1.Taint

// Tolerates returns true if the pod tolerates all taints.
func (ts Taints) Tolerates(pod *v1.Pod) (errs error) {
func (ts Taints) Tolerates(pod *corev1.Pod) (errs error) {
for i := range ts {
taint := ts[i]
tolerates := false
Expand All @@ -54,11 +57,11 @@ func (ts Taints) Tolerates(pod *v1.Pod) (errs error) {

// Merge merges in taints with the passed in taints.
func (ts Taints) Merge(with Taints) Taints {
res := lo.Map(ts, func(t v1.Taint, _ int) v1.Taint {
res := lo.Map(ts, func(t corev1.Taint, _ int) corev1.Taint {
return t
})
for _, taint := range with {
if _, ok := lo.Find(res, func(t v1.Taint) bool {
if _, ok := lo.Find(res, func(t corev1.Taint) bool {
return taint.MatchTaint(&t)
}); !ok {
res = append(res, taint)
Expand Down
2 changes: 1 addition & 1 deletion pkg/utils/node/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ var _ = Describe("NodeUtils", func() {
Spec: v1.NodeClaimSpec{
NodeClassRef: &v1.NodeClassReference{
Kind: "NodeClassRef",
Group: "test.cloudprovider/v1",
Group: "test.cloudprovider",
Name: "default",
},
},
Expand Down

0 comments on commit c52c7c1

Please sign in to comment.