Add makefile to vagrant to install collection, receiver-mock, grafana and supports avalanche #815
Conversation
sumo-drosiek
changed the title
sumo-drosiek
requested review from
perk-sumo,
a team,
frankreno and
pmalek-sumo
and removed request for
a team
vagrant/k8s/service-monitors.yaml
Outdated
| - avalanche | ||
| endpoints: | ||
| - port: "http-avalanche" # Same as service's port name | ||
| interval: 30s |
There was a problem hiding this comment.
Same elsewhere: don't we want new line characters at the end of the file?
| access_id = dummy | ||
| access_key = dummy | ||
| name = collection | ||
| mkfile_path := $(abspath $(lastword $(MAKEFILE_LIST))) |
There was a problem hiding this comment.
Out of curiosity why do we need this construct?
There was a problem hiding this comment.
I wanted this Makefile to be executable, so need to get absolute path of the file and later abs path of the repository. I will add comments to clarify that
vagrant/README.md
Outdated
| List of other useful commands in the Makefile: | ||
| - expose-prometheus - exposes prometheus on port 9090 of virtual machine | ||
| - expose-grafana - exposes grafana on port 8080 of virtual machine | ||
| - apply-avalanche - run one pod deployment of avalanche (metrics generator) |
There was a problem hiding this comment.
Nitpick: don't we want to format the targets as code with backticks?
vagrant/Makefile
Outdated
| patch-context \ | ||
| remove-tmp \ | ||
| apply-namespace \ | ||
| apply-receiver-mock \ | ||
| create-secrets \ | ||
| grafana-dashboards \ | ||
| helm-dependencies \ | ||
| helm-upgrade \ | ||
| apply-service-monitors |
There was a problem hiding this comment.
Since this is a Makefile we need tabs but why mix it with spaces here instead of just using 2 tabs?
There was a problem hiding this comment.
Indentation in the terminal looks better with two spaces instead of eight. I tried to omit tabs but it's not possible 😞
@perk-sumo thoughts?
There was a problem hiding this comment.
I'd go with tabs to make it consistent across whole Makefile file.
vagrant/Makefile
Outdated
| --namespace "${namespace}" \ | ||
| --install \ | ||
| --set sumologic.accessId="${access_id}" \ | ||
| --set sumologic.accessKey="${access_key}" \ | ||
| --set prometheus-operator.prometheus.prometheusSpec.externalLabels.cluster="${cluster}" \ | ||
| --set sumologic.clusterName="${cluster}" \ | ||
| --set sumologic.endpoint="${endpoint}" \ | ||
| --set prometheus-operator.grafana.enabled=true \ | ||
| -f "${root_dir}vagrant/values.yaml" |
There was a problem hiding this comment.
Ditto here with tabs/spaces and in a couple of other places
There was a problem hiding this comment.
I think those should be in external scripts at some point. No need to do it now.
vagrant/k8s/avalanche.yaml
Outdated
| spec: | ||
| containers: | ||
| - name: avalanche | ||
| image: quay.io/freshtracks.io/avalanche:latest |
There was a problem hiding this comment.
I'd rather specify working version in here to omit any surprises.
vagrant/k8s/logs-generator.yaml
Outdated
| spec: | ||
| containers: | ||
| - name: generator | ||
| image: localhost:32000/sumologic/kubernetes-tools:local |
There was a problem hiding this comment.
I assume local registry works by default and the image is there?
There was a problem hiding this comment.
shouldn't it be sumologic/kubernetes-tools:master?
There was a problem hiding this comment.
this file shouldn't be here 🤷
There was a problem hiding this comment.
What about the yaml comments guideline as discussed in #836 ?
Do we want to use this approach in other yaml files as well?
sumo-drosiek
force-pushed
the
drosiek-vagrant-receiver-mock
branch
from
0781781
to
bb7d01b
Compare
I suppose those files are not going to be modified by the customer, so there is no need for that |
sumo-drosiek
force-pushed
the
drosiek-vagrant-receiver-mock
branch
2 times, most recently
from
0caa020
to
f2c4b69
Compare
sumo-drosiek
force-pushed
the
drosiek-vagrant-receiver-mock
branch
4 times, most recently
from
0b9fd87
to
142f139
Compare
sumo-drosiek
force-pushed
the
drosiek-vagrant-receiver-mock
branch
from
142f139
to
7609b4d
Compare
Description
Add makefile to vagrant to install collection, receiver-mock, grafana and supports avalanche
Testing performed