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

Actually use emptydir mount when persistence is disabled #3241

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

xyzzyz
Copy link

@xyzzyz xyzzyz commented Nov 15, 2024

Description

In #2124, an emptydir was mounted when persistence.enabled: false is set. However, this is not enough to make registry work: it crashloops as in issue #1998:

Configuration error: error parsing /etc/docker/registry/config.yml: no storage configuration provided

This is fixed by setting the env variable specifying storage config even if persistence is disabled:

Related Issue

Fixes #1998

Checklist before merging

@xyzzyz xyzzyz requested review from a team as code owners November 15, 2024 16:20
Copy link

netlify bot commented Nov 15, 2024

Deploy Preview for zarf-docs canceled.

Name Link
🔨 Latest commit c19a563
🔍 Latest deploy log https://app.netlify.com/sites/zarf-docs/deploys/673cd3f595d88a000804929e

In zarf-dev#2124, an emptydir was mounted when `persistence.enabled: false` is set. However, this is not
enough to make registry work: it crashloops as in issue zarf-dev#1998. This is fixed by setting the env variable
specifying storage config even if persistence is disabled.

Signed-off-by: Adam Michalik <amichalik@anduril.com>
Copy link

codecov bot commented Nov 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅


🚨 Try these New Features:

@AustinAbro321
Copy link
Contributor

Hey, really appreciate this PR, and this is a bug. The persitence.enabled block on there was added specifically with a s3 backend in mind. I'm not sure if this change would break that use case, but the team and I will need to add a test to the Zarf repo first before I'd feel comfortable with this change. Marking this as blocked for now, and I will circle back when that test is merged.

@AustinAbro321 AustinAbro321 added bug 🐞 Something isn't working blocked labels Nov 19, 2024
@xyzzyz
Copy link
Author

xyzzyz commented Nov 21, 2024

To provide some context, I use this to bootstrap Zarf on a cluster where there's no PV provisioner available. The way I do it is to install Rancher's local-path-provisioner as part of Zarf init package:

components:
  # This package moves the injector & registries binaries
  - name: zarf-injector
    required: true
    import:
      path: zarf/packages/zarf-registry

  # Creates the temporary seed-registry with ephemeral PV to install local path provisioner
  - name: zarf-seed-registry
    required: true
    import:
      path: zarf/packages/zarf-registry
    actions:
      onDeploy:
        before:
          - cmd: echo "false"
            setVariables:
              - name: REGISTRY_PVC_ENABLED


  # Creates the temporary seed local path provisioner with ephemeral PV to install local path provisioner
  - name: seed-local-c-provisioner
    required: true
    import:
      path: zarf-local-path-provisioner

  # Creates the permanent registry
  - name: zarf-registry
    required: true
    import:
      path: zarf/packages/zarf-registry
    actions:
      onDeploy:
        before:
          - cmd: echo "true"
            setVariables:
              - name: REGISTRY_PVC_ENABLED

  # Reinstall local path provisioner to get its image inserted into permanent registry
  - name: local-path-provisioner
    required: true
    import:
      path: zarf-local-path-provisioner

  # Creates the pod+git mutating webhook
  - name: zarf-agent
    required: true
    import:
      path: zarf/packages/zarf-agent

Without this, I'd need to make local-path-provisioner installation part of pre-Zarf process.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked bug 🐞 Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

zarf init fails on multi-node minikube cluster
2 participants