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

Updated kit pipelines for kubeconfig workspaces #385

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ashishranjan738
Copy link
Contributor

This commit updates kit pipelines for kubeconfig workspaces.

Issue #, if available:

Description of changes:

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

This commit updates kit pipelines for kubeconfig workspaces.

Signed-off-by: Ashish Ranjan <[email protected]>
@@ -0,0 +1,40 @@
apiVersion: tekton.dev/v1beta1
kind: PipelineRun
Copy link
Contributor

Choose a reason for hiding this comment

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

Keep this pipelinerun here will be automatically applied to Infra cluster due to FLUX sync. Can we move this example outside of tests dir ?

- name: config
workspace: config

# finally:
Copy link
Contributor

Choose a reason for hiding this comment

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

why is this commented out ?

mountPath: /config/
steps:
- name: setup-control-plane
image: bitnami/kubectl:1.24.5 # curl was removed in more recent versions
Copy link
Contributor

Choose a reason for hiding this comment

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

can't we use Alpine regular image that we use in other tasks ?

kubectl create namespace $(params.cluster-name)
fi
echo "Setting up control plane"
cat <<EOF > /tmp/controlplane.yaml
Copy link
Contributor

@hakuna-matatah hakuna-matatah Mar 20, 2023

Choose a reason for hiding this comment

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

let's take these files as params which will reduce lot's of params in these tasks and boils down to file locations, it's much cleaner that way and also it's much more dynamic than embedding it here. Example - like how we take it for service role tasks etc.

# sanity check to ensure it's live.
kubectl --kubeconfig $(workspaces.config.path)/kubeconfig version
- name: install-cni
image: bitnami/kubectl
Copy link
Contributor

Choose a reason for hiding this comment

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

same

dataplane_name=$dp_name-$1
EC2_INSTANCES=$3
cat <<EOF >> /tmp/dp.yaml
apiVersion: kit.k8s.sh/v1alpha1
Copy link
Contributor

Choose a reason for hiding this comment

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

let's not embed these things and give flexibility to user to pass in as file location be it github or s3 whatever makes most sense.

metadata:
name: label-and-taint-node
namespace: tekton-pipelines
annotations:
Copy link
Contributor

@hakuna-matatah hakuna-matatah Mar 20, 2023

Choose a reason for hiding this comment

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

I don't think we need this annotations. it's hard to maintain as we change versions etc. Let's keep it simple and cut down as much or trim it to zero if possible.

- name: KUBECONFIG
value: /config/kubeconfig
steps:
- name: setup-data-plane
Copy link
Contributor

Choose a reason for hiding this comment

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

step name doesn't make sense for the task. Also, can we not have this as separate task and add it as a step to setup task ? Or do we have any reason to separate out this as it's own Task ?

@hakuna-matatah
Copy link
Contributor

hakuna-matatah commented Mar 20, 2023

Also can we close this draft PR then #352 as you are covering it here ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants