-
Notifications
You must be signed in to change notification settings - Fork 23
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
RHTAPINST-49: OpenShift Pipelines Configuration #44
Conversation
charts/rhtap-infrastructure/templates/openshift-pipelines/job-tekton-chains.yaml
Outdated
Show resolved
Hide resolved
charts/rhtap-infrastructure/templates/openshift-pipelines/job-tekton-chains.yaml
Outdated
Show resolved
Hide resolved
charts/rhtap-infrastructure/templates/openshift-pipelines/job-tekton-config.yaml
Show resolved
Hide resolved
@@ -0,0 +1,28 @@ | |||
{{- if .Values.infrastructure.openShiftPipelines.enabled }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This resource feels superfluous.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's the link with RHDH toggle. So, when RHDH is disabled we can also disable OpenShift Pipelines changes.
# Labels to add to the resource. | ||
labels: {} | ||
# Tekton Chains settings. | ||
tektonChains: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This section can go away, I don't see it's value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That section controls Tekton Chains related settings, and replaces hook scripts. In fact, is quite relevant.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think any of these values are controlled by the user.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These attributes have defaults, they should be good enough to work with the current version of OpenShift Pipelines, and allow us to modify as we move forward. The entries added to the values.yaml
are, in fact, variables that we use in multiple resources to avoid duplication, among other reasons to define them.
Additionally, by having these attributes we can leverage their relationship via values.yaml.tpl
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the record, I moved the defaults to the values.yaml
on rhtap-infrastructure
instead of setting them up on values.yaml.tpl
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please rebase to simplify the review.
charts/rhtap-infrastructure/templates/openshift-pipelines/job-tekton-config.yaml
Show resolved
Hide resolved
# Labels to add to the resource. | ||
labels: {} | ||
# Tekton Chains settings. | ||
tektonChains: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think any of these values are controlled by the user.
charts/values.yaml.tpl
Outdated
meta.helm.sh/release-namespace: {{ $argoCDNamespace }} | ||
labels: | ||
app.kubernetes.io/managed-by: Helm | ||
tektonChains: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's discuss tomorrow why that's needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's needed to replace this:
rhtap-cli/charts/rhtap-backing-services/hooks/pre-deploy.sh
Lines 6 to 17 in 2cff2c5
make_helm_managed() { | |
KIND="$1" | |
NAMESPACE="$2" | |
NAME="$3" | |
echo -n "." | |
kubectl annotate "$KIND" "$NAME" meta.helm.sh/release-name=rhtap-backing-services >/dev/null | |
echo -n "." | |
kubectl annotate "$KIND" "$NAME" meta.helm.sh/release-namespace="$NAMESPACE" >/dev/null | |
echo -n "." | |
kubectl label "$KIND" "$NAME" app.kubernetes.io/managed-by=Helm >/dev/null | |
} |
Thus we can run these type of interactions in the cluster, without the need for extra local tools to run rhtap-cli
.
@otaviof: The following test finished, in case test failed say /retest to rerun all failed tests:
To inspect your test artifacts make sure you have installed ORAS in your local machine. To inspect all test artifacts execute: mkdir -p oras-artifacts
cd oras-artifacts
oras pull quay.io/konflux-qe-incubator/konflux-qe-oci-storage:rhtap-cli-nt6jt-g6jz8 For instructions on how to install ORAS, please refer to the ORAS installation guide. |
Sure! Just rebased, please consider. |
5dd954d
to
0b596a0
Compare
@otaviof: The following test finished, in case test failed say /retest to rerun all failed tests:
To inspect your test artifacts make sure you have installed ORAS in your local machine. To inspect all test artifacts execute: mkdir -p oras-artifacts
cd oras-artifacts
oras pull quay.io/konflux-qe-incubator/konflux-qe-oci-storage:rhtap-cli-zkzkj-bzkgz For instructions on how to install ORAS, please refer to the ORAS installation guide. |
Moving the Tekton Chains configuration to the infrastructure chart, replacing the pre-deploy hook with a post-deploy job that will create the Tekton Chains resources. Improved the Helm chart testing to wait for OpenShift Pipelines to be ready, and using official Chainguard images to run `cosign.`
@otaviof: The following test finished, in case test failed say /retest to rerun all failed tests:
To inspect your test artifacts make sure you have installed ORAS in your local machine. To inspect all test artifacts execute: mkdir -p oras-artifacts
cd oras-artifacts
oras pull quay.io/konflux-qe-incubator/konflux-qe-oci-storage:rhtap-cli-qrvrp-zfdf6 For instructions on how to install ORAS, please refer to the ORAS installation guide. |
Moving the Tekton Chains configuration to the infrastructure chart, replacing the pre-deploy hook with a post-deploy job that will create the Tekton Chains resources.
Improved the Helm chart testing to wait for OpenShift Pipelines to be ready, and using official Chainguard images to run
cosign.
After #43 and #42 🙏