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

Dapr invoke hr api from the slack Api #13

Merged
merged 13 commits into from
Apr 19, 2021
Merged

Dapr invoke hr api from the slack Api #13

merged 13 commits into from
Apr 19, 2021

Conversation

ceddlyburge
Copy link
Contributor

The 'ping' endpoint on the slack api no longer returns 'pong' and instead uses the dapr sidecar to invoke the 'ping' endpoint on the hr api (which also returns pong)'. If working, the slack endpoint will now return 'Respose from hr api: pong'.

To test, follow the '## WIP: Deploy Slack and Hr service locally' instructions in the readme, up to and including 'Check the ping endpoint works (http://localhost/ping)' (we don't need to use dapr invoke from outside the cluster for this test, so don't need to worry about ingress).

There are now kubernetes / kustomize files for local deployment (which assumes that the docker images are available locally) and production deployment (which assumes that the docker images already exist in the github container registry)

This requires a change to the makefile (that updates the kubernetes definition files with the sha of the latest image pushed to the registry).

To test, run make ci service=hr tag=ceddtag digest=cedddigest, and check that the changes to ./manifests/overlays/production.yaml look sensible.

There is still quite a lot of debugging type stuff in the readme and some of the kubernetes definition files, which I think will be helpful in the short term, but will probably be removed once things are more stable.

This works locally, so this change replicates the previous config, but
with an overlay for the production config. This smoothes the way for
another overlay for development

Still need to work out a local configuration, and update the part of the
build that edits the kustomization.yaml files
This all works locally, so now I can make changes to the hr and slack
apps and test they work locally in k8s before merging
This is tested on a 'docker'desktop' local cluster
`kustomize edit` converts `bases` to `resources`, so better to use that
always.

`kustomize edit` doesn't like the multiple `images` sections, so
collapsing to a single list
@ceddlyburge ceddlyburge requested review from ysdexlic and alsuren April 16, 2021 11:59
Copy link
Contributor

@ysdexlic ysdexlic left a comment

Choose a reason for hiding this comment

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

might be nicer to keep the service overlays next to their own base/ directories. That way we could apply individual services in any overlay.

ceddlyburge and others added 6 commits April 16, 2021 15:26
Co-authored-by: David Thompson <david.arthur.thompson@gmail.com>
Co-authored-by: David Laban <david.laban@red-badger.com>
@ceddlyburge
Copy link
Contributor Author

might be nicer to keep the service overlays next to their own base/ directories. That way we could apply individual services in any overlay.

Hi David, I'm not sure about this, let me explain my reasoning, and then we can have a think.

  1. Currently there are two overlays, one for production and one for development (or one per environment more generally). If we have per service overlays then we need 4 (dev hr, prod hr, dev slack, prod slack), or more generally environments * services.

  2. Environments seem like the most sensible categorisation for overlays. When applying to an environment you will want everything to be kustomized for that environment. I can't think of any situations off hand where you would want to kustomize some things for one environment and some for another.

@ceddlyburge ceddlyburge merged commit f3c8434 into master Apr 19, 2021
@ceddlyburge ceddlyburge deleted the slack-invoke-hr branch April 19, 2021 18:12
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.

3 participants