Skip to content

Install monitoring-satellite with obs-installer #12701

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

Merged
merged 1 commit into from
Sep 9, 2022
Merged

Conversation

vulkoingim
Copy link
Contributor

@vulkoingim vulkoingim commented Sep 6, 2022

Description

Related Issue(s)

Fixes https://github.com/gitpod-io/ops/issues/3585

How to test

werft run github -j .werft/build.yaml -a with-preview=true

https://werft.gitpod-dev.com/job/gitpod-custom-aa-obs-installer-ss.11 (fixed the popd issue with the last commit)

NAME                                 READY   STATUS    RESTARTS   AGE
alertmanager-main-0                  2/2     Running   0          38m
grafana-8dc9c7bb9-x5525              1/1     Running   0          2m20s
http-prober-867c755dd-drgkc          1/1     Running   0          38m
kube-state-metrics-876578749-4g8t2   3/3     Running   0          38m
kubescape-bdd7f8776-lkqlw            1/1     Running   0          38m
node-exporter-xnwlh                  2/2     Running   0          38m
prometheus-k8s-0                     2/2     Running   0          38m
prometheus-operator-7f5444cc-v5x6m   2/2     Running   0          38m
pyrra-api-7944b86bfd-swrds           1/1     Running   0          38m
pyrra-kubernetes-7dd5d8dd46-xqnzd    1/1     Running   0          38m

Release Notes

NONE

Werft options:

  • /werft with-preview

@werft-gitpod-dev-com
Copy link

started the job as gitpod-build-aa-obs-installer-ss.1 because the annotations in the pull request description changed
(with .werft/ from main)


obsDir="${SCRIPT_PATH}/observability"
mkdir -p "${obsDir}"
git clone https://roboquat:"$(cat /mnt/secrets/monitoring-satellite-preview-token/token)"@github.com/gitpod-io/observability.git "${obsDir}"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We should replace this with go install, if this fixes the issue #12703

@vulkoingim vulkoingim marked this pull request as ready for review September 6, 2022 18:13
@vulkoingim vulkoingim requested a review from a team September 6, 2022 18:13
@ArthurSens
Copy link
Contributor

ArthurSens commented Sep 6, 2022

Hey aleks, I couldn't really understand what problems we're solving by using shell instead of typescript here 🤔

Could you elaborate a bit?

@meysholdt
Copy link
Member

meysholdt commented Sep 6, 2022

Hey aleks, I couldn't really understand what problems we're solving by using shell instead of typescript here 🤔

I assume that's because it's easier to test: run the bash script from your workspace instead of running the full werft job. That's in the spirit of the discussion we had in the platform sync on Monday: Have reusable modules that can be executed individually rather than one looong spaghetti. With that being said, it does not need to be bash, it just needs to be decoupled from the job.

@vulkoingim
Copy link
Contributor Author

vulkoingim commented Sep 7, 2022

In essence the server-side apply fixes the issue with the client responding that resources exist (as that check is done client-side).

As to why use shell, instead of typescript - there are a couple of reasons:

  • As Mo said - small reusable pieces that we can execute and test individually
  • Wrapping bash in typescript brings 0 benefits and makes no sense IMO
  • ☝️ makes everything more difficult to read and follow - i.e. spaghetti
  • While bash is not great - I would argue that it's much cleaner and easier to understand what this script does and how it does it exactly
  • There is a similar script in ops that spins up monitoring-satellite in ws clusters. If the issue I referenced above is fixed - then we can have that script only in a single place (here), and reduce the amount of things we have to maintain.

@vulkoingim vulkoingim force-pushed the aa/obs-installer-ss branch 2 times, most recently from f1bd9bf to 2f18757 Compare September 9, 2022 06:56
@vulkoingim
Copy link
Contributor Author

vulkoingim commented Sep 9, 2022

Here's the custom job: https://werft.gitpod-dev.com/job/gitpod-custom-aa-obs-installer-ss.16

/werft run

👍 started the job as gitpod-build-aa-obs-installer-ss.12
(with .werft/ from main)

@roboquat roboquat merged commit 19455a4 into main Sep 9, 2022
@roboquat roboquat deleted the aa/obs-installer-ss branch September 9, 2022 11:06
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.

4 participants