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

🌱 Add kubefwd and local-dev target #1034

Closed

Conversation

perdasilva
Copy link
Contributor

@perdasilva perdasilva commented Jul 11, 2024

Description

Adds:

  • kubefwd to bingo
  • hack/tool to copy the service certificates from the cluster
  • add a hack/tool script

It's helpful to be able to run/debug operator-controller directly from the IDE. Tilt is pretty nifty, but too many moving parts for me. I just want to click debug on the IDE and be done.

./hack/tools/bootstrap-local-dev
Scaling down operator-controller-controller-manager deployment
deployment.apps/operator-controller-controller-manager scaled
Copying operator-controller system namespace service certificates from kind cluster
set --ca-certs-dir=/home/perdasilva/repos/perdasilva/operator-controller/hack/certs
Starting kubefwd - sudo is needed for this
[sudo] password for perdasilva: 
INFO[10:33:28]  _          _           __             _     
INFO[10:33:28] | | ___   _| |__   ___ / _|_      ____| |    
INFO[10:33:28] | |/ / | | | '_ \ / _ \ |_\ \ /\ / / _  |    
INFO[10:33:28] |   <| |_| | |_) |  __/  _|\ V  V / (_| |    
INFO[10:33:28] |_|\_\\__,_|_.__/ \___|_|   \_/\_/ \__,_|    
INFO[10:33:28]                                              
INFO[10:33:28] Version 0.0.0                                
INFO[10:33:28] https://github.com/txn2/kubefwd              
INFO[10:33:28]                                              
INFO[10:33:28] Press [Ctrl-C] to stop forwarding.           
INFO[10:33:28] 'cat /etc/hosts' to see all host entries.    
INFO[10:33:28] Loaded hosts file /etc/hosts                 
INFO[10:33:28] HostFile management: Original hosts backup already exists at /root/hosts.original 
INFO[10:33:28] Successfully connected context: kind-operator-controller 
WARN[10:33:28] WARNING: No Running Pods returned for service operator-controller-controller-manager-metrics-service.olmv1-system.kind-operator-controller 
INFO[10:33:28] Port-Forward:       127.1.27.1 catalogd-controller-manager-metrics-service:8443 to pod catalogd-controller-manager-5cbc444644-jdrgf:8443 
INFO[10:33:28] Port-Forward:       127.1.27.2 catalogd-catalogserver:443 to pod catalogd-controller-manager-5cbc444644-jdrgf:8083 

This way you can start operator-controller with --ca-certs-dir=hack/certs and you should be able to talk to catalogd

Reviewer Checklist

  • API Go Documentation
  • Tests: Unit Tests (and E2E Tests, if appropriate)
  • Comprehensive Commit Messages
  • Links to related GitHub Issue(s)

@perdasilva perdasilva requested a review from a team as a code owner July 11, 2024 08:34
Copy link

netlify bot commented Jul 11, 2024

Deploy Preview for olmv1 ready!

Name Link
🔨 Latest commit c5638f8
🔍 Latest deploy log https://app.netlify.com/sites/olmv1/deploys/669041b8b65ab30008ff7fc3
😎 Deploy Preview https://deploy-preview-1034--olmv1.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@perdasilva perdasilva force-pushed the perdasilva/localdev branch 2 times, most recently from c705472 to 60fbc20 Compare July 11, 2024 08:40
Copy link

codecov bot commented Jul 11, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 77.00%. Comparing base (fe591d4) to head (c5638f8).
Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1034      +/-   ##
==========================================
- Coverage   77.60%   77.00%   -0.60%     
==========================================
  Files          18       19       +1     
  Lines        1268     1357      +89     
==========================================
+ Hits          984     1045      +61     
- Misses        202      222      +20     
- Partials       82       90       +8     
Flag Coverage Δ
e2e 56.44% <ø> (ø)
unit 53.86% <ø> (+1.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@bentito
Copy link
Contributor

bentito commented Jul 11, 2024

Awesome sauce, serves as a nice demo of using bingo for conditional deps too.
/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jul 11, 2024
@joelanford
Copy link
Member

Oh this is nice. Similar to make run in the standard operator-sdk scaffolding. If I understand correctly, this ends up using the local dev user's credentials and not the operator-controller service account? How much trouble would it be to also fetch the op-con SA token and use that?

@joelanford
Copy link
Member

Is the primary use case here to be able to hook up the debugger? Outside of that, it seems like our existing Tilt setup covers most of the local dev ground. My only 👎 is that this will be a second local-dev option.

I know our Tilt configuration has a delve container, so maybe it is possible to hook that up with the IDE?

@joelanford
Copy link
Member

so maybe it is possible to hook that up with the IDE?

Yeah, looks like we run all the OLM pods with a delve entrypoint and forward the delve port out to the local environment: https://github.com/operator-framework/tilt-support/blob/255e8f7a550634d40c4efc0c9844c278c67135a4/Tiltfile#L148

Comment on lines +12 to +24
rm -rf "${CERT_DIR}"
mkdir -p "${CERT_DIR}"
function import_certs() {
local secret_name=$1
data=$(kubectl get secret "${secret_name}" -n "${OLMV1_NAMESPACE}" -o jsonpath="{.data['tls\.crt']}")
if [[ -n "${data}" ]]; then
echo "${data}" | base64 --decode > "${CERT_DIR}/${secret_name}-tls.crt"
fi
}

for secret in $(kubectl get secrets -n "${OLMV1_NAMESPACE}" -o jsonpath="{.items[*].metadata.name}"); do
import_certs "${secret}"
done
Copy link
Contributor

@tmshort tmshort Jul 11, 2024

Choose a reason for hiding this comment

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

What are you doing with this certificate?
In the latest incarnation, you should just be able to grab ca.crt from the secret (if present) as the root for all certificates used between operator-controller and catalogd (at least the catalogd service). You shouldn't have to grab tls.crt, which applies only to that certificate.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just copy the certificates to a folder and start operator-controller with --ca-certs-dir so that I can talk to the catalogd endpoint and debug directly from the IDE

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The ca can change, right? and if I have the tls cert it should work every time - even if the ca changes?

Makefile Outdated Show resolved Hide resolved
Copy link
Contributor

@tmshort tmshort left a comment

Choose a reason for hiding this comment

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

A few comments.

Makefile Outdated Show resolved Hide resolved
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Jul 11, 2024
Copy link

openshift-ci bot commented Jul 11, 2024

New changes are detected. LGTM label has been removed.

Per Goncalves da Silva added 2 commits July 11, 2024 22:33
Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com>
Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com>
@perdasilva perdasilva closed this Jul 16, 2024
auto-merge was automatically disabled July 16, 2024 06:44

Pull request was closed

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.

None yet

4 participants