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

Update common #96

Merged
merged 117 commits into from
Jul 8, 2023
Merged

Update common #96

merged 117 commits into from
Jul 8, 2023

Conversation

day0hero
Copy link
Contributor

@day0hero day0hero commented Jul 8, 2023

updated common in the pattern

claudiol and others added 30 commits March 21, 2023 10:06
…annotations:

      labels:
        openshift.io/node-selector: ""
      annotations:
        openshift.io/cluster-monitoring: "true"
When running the `pattern.sh` script multiple times, a lot of
podman exited containers will be left on the machine, adding
`--rm` parameter to `podman run` makes podman automatically
delete the exited containers leaving the machine cleaner.
…ontainers

Avoid exited containers proliferation using pattern.sh script
Generating the ICSP and allowing insecure registries is best done prior
to helm upgrade, and requires VPN access to registry-proxy.engineering.redhat.com
WIP: Add labels and annotations support for namespaces (RFE)
This is especially useful when multiple people are working on a pattern
an have been using different names:

    $ make help |grep Pattern:
    Pattern: multicloud-gitops
    $ make NAME=foobar help |grep Pattern:
    Pattern: foobar
Allow overriding the pattern's name
After merging validatedpatterns/patterns-operator@235b303
it is now effectively possible to pick a different catalogSource for
the gitops operator. This is needed in order to allow CI to install
the gitops operator from an IIB
This variable can be set in order to pass additional helm arguments from the
the command line. I.e. we can set things without having to tweak values files
So it is now possible to run something like the following:

  ./pattern.sh make install \
  EXTRA_HELM_OPTS="--set main.gitops.operatorSource=iib-49232"
mbaldessari and others added 28 commits May 30, 2023 09:59
Add support for extraParams being passed down to all applications
Add a lookup playbook to figure out IIB numbers
…ator

This will allow us to test the patterns-operator using a different
catalogsource (potentially installed via an IIB). So we can run:

make EXTRA_HELM_OPTS="\
  --set main.extraParameters[0].name=main.patternsOperator.channel --set main.extraParameters[0].value=slow \
  --set main.extraParameters[1].name=main.patternsOperator.source --set main.extraParameters[1].value=patten-index" install
Allow overriding channel and source when installing the patterns-operator
Fix small typo in iib instructions
Drop a redirect and up retries when pushing the IIB to the internal registry
This reverts commit 64e9dc7.
This reverts commit ab5532a.
ArgoCD will retry 5 times by default to sync an application in case of
errors and then will give up. So if an application contains a reference
to a CRD that has not been installed yet (say because it will be
installed by another application), it will error out and retry later.
This happens by default for a maximum of 5 times [1]. After those 5 times
the application will give up and will stay in Degraded moded and
eventually move to Failed. In this case a manual sync will usually fix
the application just fine (i.e. as long as the missing CRD has been
installed in the meantime).

Now to solve this issue we can add complex preSync Jobs that wait for
the needed resources, but this fundamentally breaks the simplicity of
things and introduces unneeded dependencies. In this change we just
increase the default retry limit to something larger (20) that should
cover most cases. The retry limit functionality is rather undocumented
currently in the docs but is defined at [2] and also shown at [3].

In our patterns' case the concrete issue happened as follows:
1. ESO ClusterSecrets were often not synced/degraded
2. We introduced a Job in a preSync hook for the ESO chart that would
   wait on vault to be ready before applying the rest of ESO
3. MCG started failing because the config-demo app had already tried to
   sync 5 times and failed everytime because the ESO CRDs were not
   installed yet (due to ESO waiting on vault)

So instead of adding yet another job, let's just try a lot more often.
We picked 20 as a sane default because that should have argo try for
about 60 minutes (3min is the default maximum backoff limit)

Tested with two MCG installations (with the ESO Job hook included) and
both worked out of the box. Whereas before I managed to get three
failures out of three installs.

[1] https://github.com/argoproj/argo-cd/blob/master/controller/appcontroller.go#L1680
[2] https://github.com/argoproj/argo-cd/blob/master/manifests/crds/application-crd.yaml#L1476
[3] https://github.com/argoproj/argo-cd/blob/master/docs/operator-manual/application.yaml#L202C18-L202C100
Revert ESO/Vault Job and add a default higher number of retries
git-subtree-dir: common
git-subtree-mainline: 90cbc7e
git-subtree-split: 0c1d103
@day0hero day0hero merged commit cf3e687 into validatedpatterns:main Jul 8, 2023
2 checks passed
@day0hero day0hero deleted the update-common branch July 8, 2023 20:38
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.

5 participants