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

Support extraPorts in the service template. #154

Merged
merged 3 commits into from
Jun 4, 2024

Conversation

samwho
Copy link
Contributor

@samwho samwho commented May 31, 2024

What this PR does / why we need it:

At Budibase we're making use of the CouchDB structured query service (SQS) and as such need to communicate with the SQS binary running alongside CouchDB on port 4984. We've created our own budibase/couchdb image to run SQS alongside CouchDB.

To allow this, I've added two new options to values.yaml:

  1. extraPorts -- to specify new ports to expose on the StatefulSet pods.
  2. service.extraPorts -- to specify new ports to expose on the Service.

Special notes for your reviewer:

Before starting on this PR I ran make test and the tests passed. Now, though, when I run them I get the following error:

❯ make test
./test/e2e-kind.sh
Running ct container...
27098003d07177e9e3896e11506fe29a905fd7dcfc4fccc3add5a3b41bfe0368

Deleting cluster "chart-testing" ...
Creating cluster "chart-testing" ...
 ✓ Ensuring node image (kindest/node:v1.25.3) 🖼
 ✓ Preparing nodes 📦 📦  
 ✓ Writing configuration 📜 
 ✓ Starting control-plane 🕹️ 
 ✓ Installing CNI 🔌 
 ✓ Installing StorageClass 💾 
 ✓ Joining worker nodes 🚜 
 ✓ Waiting ≤ 1m0s for control-plane = Ready ⏳ 
 • Ready after 0s 💚
Set kubectl context to "kind-chart-testing"
You can now use your cluster with:

kubectl cluster-info --context kind-chart-testing

Thanks for using kind! 😊
Copying kubeconfig to container...
Successfully copied 7.68kB to ct:/root/.kube/config
Kubernetes control plane is running at https://127.0.0.1:56353
CoreDNS is running at https://127.0.0.1:56353/api/v1/namespaces/kube-system/services/kube-dns:dns/proxy

To further debug and diagnose cluster problems, use 'kubectl cluster-info dump'.

NAME                          STATUS     ROLES           AGE   VERSION
chart-testing-control-plane   Ready      control-plane   29s   v1.25.3
chart-testing-worker          NotReady   <none>          5s    v1.25.3

Cluster ready!

Linting and installing charts...
Version increment checking disabled.

------------------------------------------------------------------------------------------------------------------------
 Charts to be processed:
------------------------------------------------------------------------------------------------------------------------
 couchdb => (version: "4.5.4", path: "couchdb")
------------------------------------------------------------------------------------------------------------------------

Error: failed linting and installing charts: failed identifying merge base: must be in a git repository

------------------------------------------------------------------------------------------------------------------------
No chart changes detected.
------------------------------------------------------------------------------------------------------------------------
failed linting and installing charts: failed identifying merge base: must be in a git repository
Removing ct container...
Deleting cluster "chart-testing" ...
Deleted nodes: ["chart-testing-worker" "chart-testing-control-plane"]
Done!
make: *** [test] Error 1

I'm not sure what I'm doing wrong, help would be appreciated 🙏

Checklist

[Place an '[x]' (no spaces) in all applicable fields. Please remove unrelated fields.

  • Chart Version bumped
  • e2e tests pass
  • Variables are documented in the README.md
  • NEWS.md updated

@samwho
Copy link
Contributor Author

samwho commented Jun 3, 2024

Hey @willholley, I hope you don't mind me tagging you. I know it's lame to ping open-source maintainers directly, sorry about that.

Keen to get this merged as soon as is reasonable, or if it's unlikely to happen in the near future then it'd be great to know that so we can try different approaches to the problem we're having.

Let me know how I can best help this across the line. 🙏

Copy link
Member

@willholley willholley left a comment

Choose a reason for hiding this comment

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

thanks @samwho - lgtm!

@willholley willholley merged commit fa32ff8 into apache:main Jun 4, 2024
2 checks passed
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