devfile / devworkspace-operator

Apache License 2.0
59 stars 49 forks source link

Disable automount resources for the `init-container-home` init container #1273

Closed dkwon17 closed 1 week ago

dkwon17 commented 2 weeks ago

What does this PR do?

Disable automount resources for the init-persistent-home initcontainer (an not the other initcontainers) if persistent home is enabled.

What issues does this PR fix or reference?

https://github.com/devfile/devworkspace-operator/issues/1257

Is it tested? How?

  1. Deploy DWO:

    export DWO_IMG=quay.io/dkwon17/devworkspace-controller:init-container-mount
    make install
  2. In the DWOC named devworkspace-operator-config, enable persistent storage.

    apiVersion: controller.devfile.io/v1alpha1
    kind: DevWorkspaceOperatorConfig
    metadata:
    name: devworkspace-operator-config
    namespace: eclipse-che
    config:
    workspace:
    persistUserHome:
      enabled: true
  3. Create a configmap that mounts to any directory:

    kind: ConfigMap
    apiVersion: v1
    metadata:
    name: storageconf
    labels:
    controller.devfile.io/mount-to-devworkspace: 'true'
    controller.devfile.io/watch-configmap: 'true'
    annotations:
    controller.devfile.io/mount-as: subpath
    controller.devfile.io/mount-path: /home/user/.config/containers
    data:
    storage.conf: test1234
  4. Create a workspace and wait for it to start:

    oc apply -f ./samples/code-latest.yaml
  5. Verify that the first initcontainer is init-persistent-home, and that the configmap is mounted in all containers except for init-persistent-home:

    
    $ oc get pod <pod name> -o jsonpath='{.spec.initContainers[0].name}'
    init-persistent-home

$ oc get pod -o jsonpath='{.spec.initContainers[0].volumeMounts}' | grep storageconf // no match exists

$ oc get pod -o jsonpath='{.spec.initContainers[1].volumeMounts}' | grep storageconf // match exists

$ oc get pod -o jsonpath='{.spec.initContainers[2].volumeMounts}' | grep storageconf // match exists

$ oc get pod -o jsonpath='{.spec.containers[*].volumeMounts}' | grep storageconf // match exists


6. Verify that stow has run successfully in the `dev` container:

sh-4.4$ cat ~/.stow.log | tail -n 10 LINK: go/pkg/mod/mvdan.cc/xurls/v2@v2.5.0/xurls_test.go => ../../../../../../../tooling/go/pkg/mod/mvdan.cc/xurls/v2@v2.5.0/xurls_test.go level of go/pkg/sumdb is 2 MKDIR: go/pkg/sumdb level of go/pkg/sumdb/sum.golang.org is 3 MKDIR: go/pkg/sumdb/sum.golang.org level of go/pkg/sumdb/sum.golang.org/latest is 4 LINK: go/pkg/sumdb/sum.golang.org/latest => ../../../../../tooling/go/pkg/sumdb/sum.golang.org/latest Planning stow of package .... done Processing tasks... Processing tasks... done



### PR Checklist

- [ ] E2E tests pass (when PR is ready, comment `/test v8-devworkspace-operator-e2e, v8-che-happy-path` to trigger)
    - [ ] `v8-devworkspace-operator-e2e`: DevWorkspace e2e test
    - [ ] `v8-che-happy-path`: Happy path for verification integration with Che
openshift-ci[bot] commented 2 weeks ago

Skipping CI for Draft Pull Request. If you want CI signal for your change, please convert it to an actual PR. You can still manually trigger a test run with /test all

dkwon17 commented 2 weeks ago

Hello @AObuchow could you please take a look? I just want to make sure the code is OK before I add new tests

codecov[bot] commented 2 weeks ago

Codecov Report

Attention: Patch coverage is 78.94737% with 4 lines in your changes missing coverage. Please review.

Project coverage is 52.84%. Comparing base (adc8b5a) to head (1a5164c). Report is 26 commits behind head on main.

:exclamation: Current head 1a5164c differs from pull request most recent head f83b0ba

Please upload reports for the commit f83b0ba to get more accurate results.

Files Patch % Lines
pkg/library/lifecycle/prestart.go 50.00% 3 Missing and 1 partial :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #1273 +/- ## ========================================== + Coverage 52.52% 52.84% +0.32% ========================================== Files 84 84 Lines 7642 7864 +222 ========================================== + Hits 4014 4156 +142 - Misses 3335 3408 +73 - Partials 293 300 +7 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

AObuchow commented 1 week ago

@dkwon17 In the testing instructions, you said to grep for my-settings instead of storageconf: oc get pod <pod name> -o jsonpath='{.spec.containers[*].volumeMounts}' | grep my-settings. Was that intentional?

dkwon17 commented 1 week ago

@AObuchow sorry that was a mistake, it should be storageconf

AObuchow commented 1 week ago

@dkwon17 also before merging please squash your fixup commits & mention in the "Add test" commit that the tests are related to automount functionality with home persistence.

openshift-ci[bot] commented 1 week ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: AObuchow, dkwon17, ibuziuk

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files: - ~~[OWNERS](https://github.com/devfile/devworkspace-operator/blob/main/OWNERS)~~ [AObuchow,dkwon17] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment