Skip to content

Conversation

@maelvls
Copy link
Member

@maelvls maelvls commented Feb 10, 2021

In this PR, I add the ubbagent to the "jetstacksecure-mp" Helm chart.

Closes #12

Notes:

  • Right now I do not run the ubbagent as a container inside the cert-manager Pod, which should be done as detailed in billing-integration.md
  • Instead, the ubbagent runs in a standalone deployment
  • Would that be enough for this first iteration? Changing the cert-manager Deployment would require us to edit the chart, which is a tiny bit more work (but not that much I guess)

@maelvls maelvls requested review from wallrj and removed request for wallrj February 11, 2021 15:25
@maelvls maelvls self-assigned this Feb 11, 2021
@maelvls maelvls added this to the initial-release milestone Feb 11, 2021
Copy link
Member

@wallrj wallrj left a comment

Choose a reason for hiding this comment

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

Just a couple of comments and questions.
I'm going to run a cloud-build and see if I can see the billing agent deployment.

volumes:
- name: ubbagent-config
configMap:
name: ubbagent-config No newline at end of file
Copy link
Member

Choose a reason for hiding this comment

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

Missing newline

apiVersion: v1
kind: ConfigMap
metadata:
name: $name-ubbagent-config
Copy link
Member

Choose a reason for hiding this comment

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

Doesn't this need to be ubbagent-config to match the name in the deployment volume reference below?
In any case, I can't see what would replace this variable, since we're using the helm deployment mechanism rather than envsubst.
If it does get replaced, then shouldn't we use the same variable name in the deployment volume reference below?

Copy link
Member Author

Choose a reason for hiding this comment

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

Omg sorry, I just realized I pushed this PR but it was not ready at all, it seems like I forgot to remove all these $variables and replace them with Helm variables 😞

Sorry for that!

I'll ping you when I'm done fixing all that 👍

gcp:
# This parameter accepts a base64-encoded JSON service
# account key. The value comes from the reporting secret.
encodedServiceAccountKey: $AGENT_ENCODED_KEY
Copy link
Member

Choose a reason for hiding this comment

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

I don't understand where this variable gets filled in? The docs say:

Note: Use of the $AGENT_ENCODED_KEY and $AGENT_CONSUMER_ID variables in the previous example rely on variable expansion provided by the ubbagent sidecar container. The mechanism isn't supported natively by the agent. If you're using the agent in a manner other than the sidecar, you can still use the environment variable technique described above but will need to paramaterize the configuration using some other mechanism.

So perhaps there's some trick in the entrypoint of the ubbagent Docker image.

@wallrj
Copy link
Member

wallrj commented Feb 11, 2021

The cloud-build test fails for me:

tep #15 - "verify": === ERROR SUMMARY ===
Step #15 - "verify": ERROR Deployer failed
Step #15 - "verify": 
Step #15 - "verify": Error events found in namespace "apptest-giip2pag"
Step #15 - "verify": LAST SEEN   TYPE      REASON                 OBJECT                          MESSAGE
Step #15 - "verify": 5s          Warning   BackoffLimitExceeded   job/apptest-giip2pag-deployer   Job has reached the specified backoff limit
Step #15 - "verify": =====================
Step #15 - "verify": 
Step #15 - "verify": === VERIFICATION STATUS ===
Step #15 - "verify": FAILED
Step #15 - "verify": ===========================
Finished Step #15 - "verify"
ERROR
ERROR: build step 15 "gcr.io/cloud-marketplace-tools/k8s/dev" failed: step exited with non-zero status: 1
---------------------------------------------------------------------------------------------------------------------

ERROR: (gcloud.builds.submit) build b7eaf9ba-94e9-42fa-8334-321693c0e7aa completed with status "FAILURE"

It's the only way I have to test all this stuff, so it'd be good to make sure that it works, even if the ubbagent isn't completely working.

I do see various references to it in the output:

Step #15 - "verify": + echo '=== values.yaml ==='                                                                    
Step #15 - "verify": + /bin/print_config.py --output=yaml                                                            
Step #15 - "verify": === values.yaml ===               
Step #15 - "verify": cert-manager:                                                                                   
Step #15 - "verify":   cainjector:                                                                                   
Step #15 - "verify":     serviceAccount:                  
Step #15 - "verify":       name: apptest-giip2pag-cert-manager-cainjector-serviceaccount-name                        
Step #15 - "verify":   image:          
Step #15 - "verify":     repository: gcr.io/jetstack-richard/cert-manager/cert-manager-controller
Step #15 - "verify":     tag: 1.1.0       
Step #15 - "verify":   serviceAccount: 
Step #15 - "verify":     name: apptest-giip2pag-cert-manager-serviceaccount-name 
Step #15 - "verify":   webhook:                          
Step #15 - "verify":     serviceAccount:  
Step #15 - "verify":       name: apptest-giip2pag-cert-manager-webhook-serviceaccount-name
Step #15 - "verify": name: apptest-giip2pag                                                                          
Step #15 - "verify": namespace: apptest-giip2pag                                                                     
Step #15 - "verify": ubbagent:                                                                                       
Step #15 - "verify":   reportingSecret: fake-reporting-secret-8ogmm4zk                                                                                                                                                                    
Step #15 - "verify": ===================                                                                             
Step #15 - "verify": + echo ===================           
Step #15 - "verify": + for chart in '"$data_dir/extracted"/*'
Step #15 - "verify": ++ basename /data/extracted/chart    
Step #15 - "verify": ++ sed 's/.tar.gz$//'                                                                           
Step #15 - "verify": + chart_manifest_file=chart.yaml                                                                
Step #15 - "verify": + helm template apptest-giip2pag /data/extracted/chart/chart --namespace=apptest-giip2pag --values=/dev/fd/63
Step #15 - "verify": ++ /bin/print_config.py --output=yaml                                                           
Step #15 - "verify": load.go:112: Warning: Dependencies are handled in Chart.yaml since apiVersion "v2". We recommend migrating dependencies to Chart.yaml.
Step #15 - "verify": Error: YAML parse error on jetstacksecure-mp/templates/billing-agent.yml: error converting YAML to JSON: yaml: unknown anchor 'AppDeploymentLabels' referenced
Step #15 - "verify":                                                                                                 
Step #15 - "verify": Use --debug flag to render out invalid YAML                                                     

@maelvls maelvls force-pushed the add-ubbagent-to-helm branch from 3f5bce8 to fbc906b Compare February 12, 2021 10:26
@maelvls maelvls force-pushed the add-ubbagent-to-helm branch from fbc906b to 95a177d Compare February 12, 2021 14:30
@maelvls maelvls marked this pull request as ready for review February 12, 2021 14:50
Signed-off-by: Maël Valais <[email protected]>
Co-authored-by: Richard Wall <[email protected]>
@maelvls maelvls force-pushed the add-ubbagent-to-helm branch from 95a177d to 36d856b Compare February 12, 2021 14:52
Signed-off-by: Maël Valais <[email protected]>
Co-authored-by: Richard Wall <[email protected]>
Signed-off-by: Maël Valais <[email protected]>
Co-authored-by: Richard Wall <[email protected]>
maelvls and others added 2 commits February 12, 2021 16:38
Signed-off-by: Maël Valais <[email protected]>
Co-authored-by: Richard Wall <[email protected]>
@maelvls maelvls force-pushed the add-ubbagent-to-helm branch from 90d80fc to 1e1d160 Compare February 12, 2021 16:07
maelvls and others added 2 commits February 12, 2021 17:09
Signed-off-by: Maël Valais <[email protected]>
Co-authored-by: Richard Wall <[email protected]>
Signed-off-by: Maël Valais <[email protected]>
Co-authored-by: Richard Wall <[email protected]>
Copy link
Member

@wallrj wallrj left a comment

Choose a reason for hiding this comment

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

/lgtm

identity: gcp
# The service name is unique to your application and will be
# provided during onboarding.
serviceName: my-application.mp-my-company.appspot.com
Copy link
Member

Choose a reason for hiding this comment

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

This name needs updating.

@jetstack-bot jetstack-bot merged commit 34b7a9c into main Feb 12, 2021
@maelvls maelvls deleted the add-ubbagent-to-helm branch February 12, 2021 16:28
@maelvls maelvls mentioned this pull request Apr 29, 2021
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add the hourly billing agent ubbagent to our helm chart

4 participants