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

Refactor action wait code #2203

Open
lucasrod16 opened this issue Dec 19, 2023 · 6 comments · May be fixed by #2678
Open

Refactor action wait code #2203

lucasrod16 opened this issue Dec 19, 2023 · 6 comments · May be fixed by #2678
Labels
tech-debt 💳 Debt that the team has charged and needs to repay

Comments

@lucasrod16
Copy link
Contributor

Describe what should be investigated or refactored

The code for executing action waits should be refactored to make it easier to extend

Links to any relevant code

src/pkg/utils/wait.go

Additional context

@Noxsios @AustinAbro321

Looked into adding a --silent flag to control the output of zarf tools wait-for in #2180 and found that the function signature for ExecuteWait would grow to 7 input parameters

@lucasrod16 lucasrod16 added the tech-debt 💳 Debt that the team has charged and needs to repay label Dec 19, 2023
@Racer159
Copy link
Contributor

This may be a good opportunity to look at making this a common library for things like Lula as well

@lucasrod16
Copy link
Contributor Author

@phillebaba @AustinAbro321 and I were chatting about this today.

Things to consider for this refactor:

  1. Look into using kstatus package to take advantage of upstream, well-tested logic. This would replace our current behavior of shelling out to kubectl

  2. See if there are any places to simplify our wait API. The Flux Kustomization spec.healthChecks can be used for reference/inspiration

@phillebaba
Copy link
Member

I second the point of adopting kstatus and adding a healthChecks list additionally. We had success in Flux doing it this way as it would also support health checking CRDs as long as they conformed to kstatus. Over time other projects added support for kstatus as people requested compatibility with Flux.

@Noxsios
Copy link
Contributor

Noxsios commented May 7, 2024

I'm always down for removing shell abstractions, send it

@phillebaba
Copy link
Member

I have done some exploration of the existing code now to understand any potential challenges. The main one is that we need a standard method of fetching Kubernetes configuration. Right now the CLI depends on the host machine being properly configured. We may want to add a --kubeconfig flag to allow explicitly setting this.

@phillebaba
Copy link
Member

My suggestion for addressing this is to add health checking as a feature now that we have a shared health checking pkg. This would mimic the behavior that Flux has pretty closely. My suggestion is that we add a new field to the components that allow the users to specify their health checks.

kind: ZarfPackageConfig
metadata:
  name: argocd
  description: Example showcasing installing ArgoCD
components:
  - name: argocd-helm-chart
    required: true
    charts:
      - name: argo-cd
        version: 5.54.0
        namespace: argocd
        url: https://argoproj.github.io/argo-helm
        releaseName: argocd-baseline
        valuesFiles:
          - baseline/values.yaml
    images:
      - docker.io/library/redis:7.0.15-alpine
      - quay.io/argoproj/argocd:v2.9.6
      # Cosign artifacts for images - argocd - argocd-helm-chart
      - quay.io/argoproj/argocd:sha256-2dafd800fb617ba5b16ae429e388ca140f66f88171463d23d158b372bb2fae08.sig
      - quay.io/argoproj/argocd:sha256-2dafd800fb617ba5b16ae429e388ca140f66f88171463d23d158b372bb2fae08.att
    healthChecks:
      - apiVersion: v1
        kind: Deployment
        name: argocd
        namespace: argocd

My plan is that we just use the timeout set when calling the Zarf command.

@phillebaba phillebaba linked a pull request Jun 28, 2024 that will close this issue
2 tasks
@salaxander salaxander removed this from the The Bucket milestone Jul 3, 2024
@phillebaba phillebaba self-assigned this Jul 11, 2024
@phillebaba phillebaba added this to the v0.37.0 milestone Jul 11, 2024
@phillebaba phillebaba removed this from the v0.37.0 milestone Jul 12, 2024
@phillebaba phillebaba removed their assignment Jul 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tech-debt 💳 Debt that the team has charged and needs to repay
Projects
Status: Backlog
Development

Successfully merging a pull request may close this issue.

5 participants