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

Deploy Tools using ArgoCD if installed #113

Merged
merged 16 commits into from
Jun 22, 2023

Conversation

CloudoguSiebels
Copy link
Contributor

No description provided.

CloudoguSiebels and others added 3 commits June 7, 2023 16:59
We want turn ScmmRepo into a class that is responsible for using SCMM
and want to move away from being tightly coupled with the ArgoCD class
We want to move the responsibility for managing the temporary directory
from the caller to the ScmmRepo class.
This removes the burden to create the directory and ensure deletion from the caller.
We want to simplify the callers code regarding absolute paths.

As a result, we deprecate the original constructor to move away from
passing the temporary directory to the object instance.

Co-authored-by: Johannes Schnatterer <johannes.schnatterer@cloudogu.com>
Co-authored-by: Philipp Dziosa <philipp.dziosa@cloudogu.com>
Co-authored-by: Sentürk Ugras <sentuerk.ugras-extern@cloudogu.com>
We want to deploy features using Argo CD and Helm depending on the
configuration.
Every strategy is responsible for one of these tools.
We want to abstract this logic from these features (e.g. mailhog,
prometheus).

Co-authored-by: Johannes Schnatterer <johannes.schnatterer@cloudogu.com>
Co-authored-by: Philipp Dziosa <philipp.dziosa@cloudogu.com>
Co-authored-by: Sentürk Ugras <sentuerk.ugras-extern@cloudogu.com>
@CloudoguSiebels CloudoguSiebels force-pushed the feature/deploy-tools-via-argocd branch 2 times, most recently from 8861553 to c2c80ed Compare June 15, 2023 14:47
We already have a deployment strategy using Helm.
This is the counterpart using Argo CD.
We need multi source applications otherwise.
However, these are still in beta.
We want to install Mailhog using Argo CD when it is installed.
We use the previously established concept of the Deployer
which uses different strategies (Helm, ArgoCD) to deploy tools.

In future steps, the other tools will also be migrated to this approach.
@CloudoguSiebels CloudoguSiebels force-pushed the feature/deploy-tools-via-argocd branch from c2c80ed to 1e4353e Compare June 15, 2023 15:10
Move docs from ArgoCdApplicationStrategy to README.

Deployer
- add some comments why there is no FluxStrategy -#114.
- Use concrete strategy class for members. This helps us to not accidentally set the wrong implementation.
@schnatterer
Copy link
Member

Reviewed, looks good. I like the design! I've made some minor polishing. Please have a look. Are you alright with the changes?

I have one issue and some points to discuss:

I'd like to simplify HelmValuesConverter.
At the moment the result looks like this

    helm:
      releaseName: "mailhog"
      parameters:
      - name: "service.type"
        value: "NodePort"
      - name: "service.port.http"
        value: "80"

I'm convinced this would be easier to maintain

        releaseName: "mailhog"
        values: |
          service:
            type: LoadBalancer
            port:
              http: 80

Reference
Do you agree?

Things to discuss:

  • Iterative installation of the Playground. First time with --argocd, next time with --flux will fail. Any ideas for a pragmatic solution?
[main] ERROR c.c.gitops.utils.CommandExecutor - Stderr: Error: rendered manifests contain a resource that already exists. Unable to continue with install: ServiceAccount "mailhog" in namespace "monitoring" exists and cannot be imported into the current release: invalid ownership metadata; annotation validation error: missing key "meta.helm.sh/release-name": must be set to "mailhog"; annotation validation error: missing key "meta.helm.sh/release-namespace": must be set to "monitoring"`
  • A idea to make the folder structure more intuitive, which is not necessarily related to this PR: Move system to applications/cluster-resources

@CloudoguSiebels
Copy link
Contributor Author

Are you alright with the changes?

I fixed a typo. Otherwise happy with it.

I'd like to simplify HelmValuesConverter.

Yes, absolutely. I didn't realize that this was possible.

Iterative installation of the Playground. First time with --argocd, next time with --flux will fail. Any ideas for a pragmatic solution?

Is it allowed to install the playground using different features? e.g. first without parameters, next run add --vault=dev, next run add --monitoring? If not, we could persist the parameters of the initial run and compare them in the next one.
If that is supported however, what do we expect to happen to omitted features? Do we expect argocd to be uninstalled, when we don't pass the --argocd flag in the second run (same for all other tools)? If not, we should probably remove the ArgoCD-Application from the cluster-resource repository when flux is installed. Then ArgoCD should purge them and we install them again with helm.

In this case, we probably want to add a concept of uninstalling to the strategies. That way, we would also solve that weird state where ArgoCD and Helm manage the application at the same time when upgrading from no parameters to --argocd.

Move system to applications/cluster-resources

I would not do that as long as we also support installation via helm. In that case we don't use the cluster-resources repository; and I wouldn't expect them there.

The systems folder was surprising me as well when I first encountered it. I think we do want to have a folder that is common to different parts of the playground. I think this could be solved by naming.
system contains data for our tools. We have various names for these: tools, features, system. Can we unify these? Do you have suggestions?

CloudoguSiebels and others added 5 commits June 19, 2023 13:39
We can embed the yaml directly into the application.yaml.
Therefore, we don't need to convert it into dot-notation anymore.
Co-authored-by: Tim Siebels <tim.siebels@cloudogu.com>
From /system
to /applications/cluster-resources

In order to move to a more unified wording. No longer use the word "system", but the word "cluster-resources".

Co-authored-by: Tim Siebels <tim.siebels@cloudogu.com>
And move decision about "Deploying Cluster Resources with Argo CD using inline YAML" from README to decision.

Less noise for the user, more context for developers.

Co-authored-by: Tim Siebels <tim.siebels@cloudogu.com>
Avoids dependency missmatches that eventually lead to  failing e2e tests with error:

org.jenkinsci.plugins.workflow.actions.ErrorAction$ErrorId: c8730323-d1bc-4e1c-921a-c62413072ca6
java.lang.NoSuchMethodError: No such DSL method 'junit' found among steps
@schnatterer
Copy link
Member

schnatterer commented Jun 21, 2023

We discussed and decided upon the issues mentioned above.

I then did a final manual test (successful).

While trying to finish up this PR, I stumbled on a number of issues on our build system:

  • Jenkins plugin dependency issues once again, resolved in e1ee67c
  • then Error during helm pull  gitops-build-lib#34
  • and also (which seems to happen a lot recently 🤔 ) stderr: fatal: unable to access 'http://scmm-scm-manager/scm/repo/argocd/example-apps/': Failed to connect to scmm-scm-manager port 80: Connection timed out or this:
Started by user Jenkins Admin java.net.SocketTimeoutException: connect timed out at java.base/java.net.PlainSocketImpl.socketConnect(Native Method) at java.base/java.net.AbstractPlainSocketImpl.doConnect(AbstractPlainSocketImpl.java:412) at java.base/java.net.AbstractPlainSocketImpl.connectToAddress(AbstractPlainSocketImpl.java:255) at java.base/java.net.AbstractPlainSocketImpl.connect(AbstractPlainSocketImpl.java:237) at java.base/java.net.SocksSocketImpl.connect(SocksSocketImpl.java:392) at java.base/java.net.Socket.connect(Socket.java:609) at okhttp3.internal.platform.Platform.connectSocket(Platform.kt:128) at okhttp3.internal.connection.RealConnection.connectSocket(RealConnection.kt:295) at okhttp3.internal.connection.RealConnection.connect(RealConnection.kt:207) at okhttp3.internal.connection.ExchangeFinder.findConnection(ExchangeFinder.kt:226) at okhttp3.internal.connection.ExchangeFinder.findHealthyConnection(ExchangeFinder.kt:106) at okhttp3.internal.connection.ExchangeFinder.find(ExchangeFinder.kt:74) at okhttp3.internal.connection.RealCall.initExchange$okhttp(RealCall.kt:255) at okhttp3.internal.connection.ConnectInterceptor.intercept(ConnectInterceptor.kt:32) at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.kt:109) at okhttp3.internal.cache.CacheInterceptor.intercept(CacheInterceptor.kt:95) at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.kt:109) at okhttp3.internal.http.BridgeInterceptor.intercept(BridgeInterceptor.kt:83) at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.kt:109) at okhttp3.internal.http.RetryAndFollowUpInterceptor.intercept(RetryAndFollowUpInterceptor.kt:76) at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.kt:109) at okhttp3.internal.connection.RealCall.getResponseWithInterceptorChain$okhttp(RealCall.kt:201) at okhttp3.internal.connection.RealCall$AsyncCall.run(RealCall.kt:517) at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128) at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628) at java.base/java.lang.Thread.run(Thread.java:829) Caused: java.util.concurrent.ExecutionException at java.base/java.util.concurrent.CompletableFuture.reportGet(CompletableFuture.java:395) at java.base/java.util.concurrent.CompletableFuture.get(CompletableFuture.java:1999) at com.cloudogu.scmmanager.scm.api.Futures.resolveUnchecked(Futures.java:30) Caused: java.io.IOException Caused: java.io.UncheckedIOException: failed to fetch at com.cloudogu.scmmanager.scm.api.Futures.unchecked(Futures.java:40) at com.cloudogu.scmmanager.scm.api.Futures.resolveUnchecked(Futures.java:35) at com.cloudogu.scmmanager.scm.ScmManagerSourceRetriever.create(ScmManagerSourceRetriever.java:123) at com.cloudogu.scmmanager.scm.ScmManagerSource.handleRequest(ScmManagerSource.java:127) at com.cloudogu.scmmanager.scm.ScmManagerSource.retrieve(ScmManagerSource.java:119) at jenkins.scm.api.SCMSource._retrieve(SCMSource.java:372) at jenkins.scm.api.SCMSource.retrieve(SCMSource.java:597) at jenkins.scm.api.SCMSource.fetch(SCMSource.java:581) at org.jenkinsci.plugins.workflow.multibranch.SCMBinder.create(SCMBinder.java:99) at org.jenkinsci.plugins.workflow.job.WorkflowRun.run(WorkflowRun.java:312) at hudson.model.ResourceController.execute(ResourceController.java:101) at hudson.model.Executor.run(Executor.java:442) Finished: FAILURE

@schnatterer
Copy link
Member

I recognized one more issue: #115.
As this can likely be fixed by upgrading prometheus, and we are planning to do this anyway, I created a separate issue to bring this PR to an end.

@schnatterer schnatterer merged commit 40ca6a8 into main Jun 22, 2023
1 check passed
@schnatterer schnatterer deleted the feature/deploy-tools-via-argocd branch June 22, 2023 14:46
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

2 participants