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

Test the UI and CLI user flows #34

Closed
2 tasks done
maelvls opened this issue Mar 8, 2021 · 7 comments
Closed
2 tasks done

Test the UI and CLI user flows #34

maelvls opened this issue Mar 8, 2021 · 7 comments
Assignees

Comments

@maelvls
Copy link
Member

maelvls commented Mar 8, 2021

Requirement: https://github.com/jetstack/platform-board/issues/338 (jetstack private link)

Now that the application was accepted by Google, the app is in "private" mode. People that can access the jetstack-public project can test it out by going on the application's page and using the jetstack-dev project (since Zee already "Purchased" the app on that project).

The next step is to test the two possible user flows:

This issue is about trying out both flows and reporting any issue.

@maelvls maelvls added this to the initial-release milestone Mar 8, 2021
@maelvls maelvls self-assigned this Mar 8, 2021
@maelvls
Copy link
Member Author

maelvls commented Mar 10, 2021

The UI flow has a "bug": the only component that shows in the Application page is the ubbagent. It should show the cert-manager deployment, etc

only-ubbagent-showing

The way the Application "wrangles" its components is by using a label selector (see API doc). The label selector is defined in application.yaml. For example, let us imagine the application was installed with:

kind: Application
spec:
  selector:
    matchLabels:
      app.kubernetes.io/instance: jetstack-secure-for-cert-mana-2

Then all the components (pods, deployments, services, configmaps, secrets, CRDs, mutating and validating webhooks) should also be installed with this label. I double-checked, this selector seems to be correctly "wrangling" all the components:

% gcloud container clusters get-credentials --project jetstack-dev --region europe-west1-b cluster-1
% k get -n jetstack-secure all -l app.kubernetes.io/instance=jetstack-secure-for-cert-mana-2
NAME                                                                  READY   STATUS              RESTARTS   AGE
pod/jetstack-secure-for-cert-mana-2-cert-manager-8574bf7577-j86vh     1/1     Running             0          18m
pod/jetstack-secure-for-cert-mana-2-cert-manager-cainjector-979fp28   1/1     Running             0          18m
pod/jetstack-secure-for-cert-mana-2-cert-manager-webhook-57444sjnmn   1/1     Running             0          18m
pod/jetstack-secure-for-cert-mana-2-google-cas-issuer-87cb6cf7cwgt2   1/1     Running             0          18m
pod/jetstack-secure-for-cert-mana-2-preflight-7498975f56-hdkrl        0/1     ContainerCreating   0          18m

NAME                                                           TYPE        CLUSTER-IP     EXTERNAL-IP   PORT(S)    AGE
service/jetstack-secure-for-cert-mana-2-cert-manager           ClusterIP   10.3.241.112   <none>        9402/TCP   18m
service/jetstack-secure-for-cert-mana-2-cert-manager-webhook   ClusterIP   10.3.246.183   <none>        443/TCP    18m
service/jetstack-secure-for-cert-mana-2-google-cas-issuer      ClusterIP   10.3.252.188   <none>        9402/TCP   18m

NAME                                                                      READY   UP-TO-DATE   AVAILABLE   AGE
deployment.apps/jetstack-secure-for-cert-mana-2-cert-manager              1/1     1            1           18m
deployment.apps/jetstack-secure-for-cert-mana-2-cert-manager-cainjector   1/1     1            1           18m
deployment.apps/jetstack-secure-for-cert-mana-2-cert-manager-webhook      1/1     1            1           18m
deployment.apps/jetstack-secure-for-cert-mana-2-google-cas-issuer         1/1     1            1           18m
deployment.apps/jetstack-secure-for-cert-mana-2-preflight                 0/1     1            0           18m
deployment.apps/jetstack-secure-for-cert-mana-2-ubbagent                  1/1     1            1           18m

NAME                                                                                DESIRED   CURRENT   READY   AGE
replicaset.apps/jetstack-secure-for-cert-mana-2-cert-manager-8574bf7577             1         1         1       18m
replicaset.apps/jetstack-secure-for-cert-mana-2-cert-manager-cainjector-97c544cfb   1         1         1       18m
replicaset.apps/jetstack-secure-for-cert-mana-2-cert-manager-webhook-57444bd757     1         1         1       18m
replicaset.apps/jetstack-secure-for-cert-mana-2-google-cas-issuer-87cb6cf77         1         1         1       18m
replicaset.apps/jetstack-secure-for-cert-mana-2-preflight-7498975f56                1         1         0       18m

@maelvls
Copy link
Member Author

maelvls commented Mar 10, 2021

After a bit more digging, I found out that the official Application API says to use app.kubernetes.io/name:

The selector is used to match resources that belong to the Application. All of the applications resources should have labels such that they match this selector. Users should use the app.kubernetes.io/name label on all components of the Application and set the selector to match this label. For instance,

selector:
  matchLabels:
    app.kubernetes.io/name: my-cool-app

should be used as the selector for an Application named "my-cool-app", and each component should contain a label that matches.

Without realizing that using app.kubernetes.io/name was mandatory, @wallrj and I chose to go with app.kubernetes.io/instance because the app.kubernetes.io/name in the cert-manager helm chart, defined in _helpers.tpl, defaults to the chart name instead of the release name:

app.kubernetes.io/name: {{- default .Chart.Name .Values.nameOverride | trunc 63 | trimSuffix "-" -}}

Not sure why cert-manager uses the chart name instead of the release name for the app.kubernetes.io/name...


So the solution to this "mandatory label" is just to use app.kubernetes.io/name in our application.yaml. But then, we have to find a workaround for cert-manager's Helm chart that sets app.kubernetes.io/name to the chart name...

IDEA 1: I thought about nameOverride, but we cannot use helm values in values.yaml, i.e. this is not possible:

# https://github.com/jetstack/jetstack-secure-gcm/blob/main/chart/jetstack-secure-gcm/values.yaml
cert-manager:
  nameOverride: {{ .Release.Name }}                    # ❌ not possible 

IDEA 2: I also thought about duplicationg the NAME value in our schema.yaml since NAME corresponds to the {{ .Release.Name }} variable (this is detailed in the official schema.md). But there does not seem to be a way to re-use .Release.Name anywhere anyways... Maybe by duplicating the name field, but that's just not possible (or is it?)

properties:
  name:                      # maps to Helm's Release.Name directive
    type: string
    x-google-marketplace:
      type: NAME
  cert-manager.nameOverride: # duplicate of the above `name` field
    type: string
    x-google-marketplace:
      type: NAME

IDEA 3: My last thought is to have our own custom version of the cert-manager's chart: we would stop using the upstream chart as a dependency. I think that's the only way forward! not needed, see idea 4

@maelvls
Copy link
Member Author

maelvls commented Mar 10, 2021

IDEA 4: @wallrj and I have found the reason why only the ubbagent was the only deployment that was showing up: the app.kubernetes.io/name is automatically added to the matchLabels. So we thought: why not use both name and instance? The application would look like this:

# https://github.com/jetstack/jetstack-secure-gcm/blob/main/chart/jetstack-secure-gcm/templates/application.yaml
kind: Application
spec:
  selector:
    matchLabels:
      app.kubernetes.io/name: {{ .Chart.Name }}          # Will always be "jetstack-secure-gcm"
      app.kubernetes.io/instance: {{ .Release.Name }}    # Example: "jetstack-secure-for-cert-mana-2"

First, we set the name override for all our charts:

# https://github.com/jetstack/jetstack-secure-gcm/blob/main/chart/jetstack-secure-gcm/values.yaml
cert-manager:
  nameOverride: jetstack-secure-gcm
google-cas-issuer:
  nameOverride: jetstack-secure-gcm
preflight:
  nameOverride: jetstack-secure-gcm

Then we make sure all the objects are set with the labels:

# All the manifests and subcharts under
# https://github.com/jetstack/jetstack-secure-gcm/blob/main/chart/jetstack-secure-gcm/templates
metadata:
  app.kubernetes.io/name: "{{ .Chart.Name }}"        # Will be "jetstack-secure-gcm" due to the name override
  app.kubernetes.io/instance: "{{ .Release.Name }}"  # Example: "jetstack-secure-for-cert-mana-2"

And everything works, all the "components" are showing in the Application UI!

@maelvls
Copy link
Member Author

maelvls commented Mar 10, 2021

Other findings... Trying to do this approach, everything seemed OK except for the cainjector:

kind: ReplicaSet
metadata:
  labels:
    app.kubernetes.io/instance: apptest-qd8oe04y
    app.kubernetes.io/name: cainjector                       # ❌ should read "jetstack-secure-gcm"

I guess I'll just ignore that for now. Just for the record, the explanation is in _helpers.tpl:

{{/*
Expand the name of the chart.
Manually fix the 'app' and 'name' labels to 'cainjector' to maintain
compatibility with the v0.9 deployment selector.
*/}}
{{- define "cainjector.name" -}}
{{- printf "cainjector" -}}
{{- end -}}

Same with the webhook...

@maelvls
Copy link
Member Author

maelvls commented Mar 10, 2021

The issue with the application wrangling was fixed in #35. We can see most components:

Screenshot 2021-03-11 at 00 37 58

@maelvls
Copy link
Member Author

maelvls commented Mar 15, 2021

The UI flow was thoroughly tested.

Regarding the CLI flow, I did not have time to test the Google CAS integration. I will be doing that as part of #19

/close

@jetstack-bot
Copy link
Contributor

@maelvls: Closing this issue.

In response to this:

The UI flow was thoroughly tested.

Regarding the CLI flow, I did not have time to test the Google CAS integration. I will be doing that as part of #19

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

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

No branches or pull requests

2 participants