runtime icon indicating copy to clipboard operation
runtime copied to clipboard

Acorn silently stops reconciliation if target-namespace existed before

Open denist-huma opened this issue 2 years ago • 4 comments

$ kubectl create ns getting-started-acorn
namespace/getting-started-acorn created
$ kubectl apply -f redis-claim.yaml -n getting-started-acorn
redisclaim.redis.operator.huma.com/redis created
$ acorn rm getting-started
getting-started
$ acorn run --name getting-started-acorn \
>     --target-namespace getting-started-acorn \
>     ghcr.io/huma-engineering/compose-operator-examples/getting-started:v0.1.0 
getting-started-acorn
$ acorn apps
NAME                    IMAGE                                                                       HEALTHY   UP-TO-DATE   CREATED   ENDPOINTS   MESSAGE
getting-started-acorn   ghcr.io/huma-engineering/compose-operator-examples/getting-started:v0.1.0                          54s ago               [controller: not processed] 
$ acorn apps
NAME                    IMAGE                                                                       HEALTHY   UP-TO-DATE   CREATED   ENDPOINTS   MESSAGE
getting-started-acorn   ghcr.io/huma-engineering/compose-operator-examples/getting-started:v0.1.0                          64s ago               [controller: not processed] 
$ acorn apps
NAME                    IMAGE                                                                       HEALTHY   UP-TO-DATE   CREATED   ENDPOINTS   MESSAGE
getting-started-acorn   ghcr.io/huma-engineering/compose-operator-examples/getting-started:v0.1.0                          68s ago               [controller: not processed] 
$ acorn apps
NAME                    IMAGE                                                                       HEALTHY   UP-TO-DATE   CREATED    ENDPOINTS   MESSAGE
getting-started-acorn   ghcr.io/huma-engineering/compose-operator-examples/getting-started:v0.1.0                          113s ago               [controller: not processed]

Versions:

  • K8s Rev: v1.23.8-gke.400
  • acorn version v0.2.0+8dd7a3bb

denist-huma avatar Sep 06 '22 11:09 denist-huma

Weird and interesting. We'll take a look

cjellick avatar Sep 06 '22 15:09 cjellick

Steps to reproduce:

Acornfile:

containers: {
  cache: {
    image: "redis:alpine"
    ports: "6379/tcp"
  }
}
  1. Create a namespace kubectl create ns ns-test-app

  2. acorn run -n test-app --target-namespace ns-test-app .

  3. acorn apps

output:

NAME       IMAGE          HEALTHY   UP-TO-DATE   CREATED     ENDPOINTS   MESSAGE
test-app   379dac6727f8                          7m52s ago               [controller: not processed] 

4 check appinstances.internal.acorn.io CRD kubectl get appinstances.internal.acorn.io -A -o yaml

output:

apiVersion: v1
items:
- apiVersion: internal.acorn.io/v1
  kind: AppInstance
  metadata:
    creationTimestamp: "2022-09-06T23:24:28Z"
    generation: 1
    labels:
      acorn.io/managed: "true"
      acorn.io/root-namespace: acorn
    name: test-app
    namespace: acorn
    resourceVersion: "1692"
    uid: 8f344447-f5f5-424f-bad4-9c3fea705d15
  spec:
    image: 379dac6727f89721283eac55f459b7538b3ba2ffe69afcabd4effe8fc121eb84
    targetNamespace: ns-test-app
  status:
    appImage:
      acornfile: |
        containers: {
          cache: {
            image: "redis:alpine"
            ports: "6379/tcp"
          }
        }
      id: 379dac6727f89721283eac55f459b7538b3ba2ffe69afcabd4effe8fc121eb84
      imageData:
        containers:
          cache:
            image: sha256:438b33ad07ea40f5c8cbbbc893b0faa2ed204dafac880c73ae6359b8f20ce17a
    appSpec:
      containers:
        cache:
          build:
            baseImage: redis:alpine
          image: sha256:438b33ad07ea40f5c8cbbbc893b0faa2ed204dafac880c73ae6359b8f20ce17a
          permissions: {}
          ports:
          - port: 6379
            protocol: tcp
            targetPort: 6379
    columns: {}
    conditions:
    - error: true
      lastTransitionTime: "2022-09-06T23:24:28Z"
      message: can not use namespace ns-test-app, existing namespace must have labels
        [ns-test-app=acorn] and [test-app=test-app]. And namespace will be deleted
        when the app is deleted
      observedGeneration: 1
      reason: Error
      status: "False"
      type: namespace
    - lastTransitionTime: "2022-09-06T23:24:28Z"
      observedGeneration: 1
      reason: Success
      status: "True"
      success: true
      type: pulled
    - lastTransitionTime: "2022-09-06T23:24:28Z"
      observedGeneration: 1
      reason: Success
      status: "True"
      success: true
      type: parsed
    - lastTransitionTime: "2022-09-06T23:24:28Z"
      observedGeneration: 1
      reason: Success
      status: "True"
      success: true
      type: controller
kind: List
metadata:
  resourceVersion: ""

meldafrawi avatar Sep 06 '22 23:09 meldafrawi

Thanks for the debugging @meldafrawi

Looking at @ibuildthecloud's PR for this feature, I guess this is behaving as expected. If the namespace is pre-existing, it has to be labelled a certain way. Specifically:

acorn.io/app-namespace=<namespace the App was created in, 'acorn' by default>"
acorn.io/app-name=<name of the app, in Mohamed's case 'test-app'>"

This is to prevent, or at least make more difficult, an unauthorized deployment into someone else's namespace by requiring the target namespace have labels that match the app you are deploying.

There is some improvement to be made here, either in docs, help text, error messages, or all of the above. I'm just not sure which yet. But @denist-huma you could actually move forward by labelling that namespace accordingly.

cjellick avatar Sep 06 '22 23:09 cjellick

It looks like the error message getting generated is wrong. Is supposed to say what labels you need. I can look at that.

ibuildthecloud avatar Sep 07 '22 05:09 ibuildthecloud

didnt make it into v0.3.0, but will get to it in 0.4.0

cjellick avatar Oct 04 '22 15:10 cjellick

@jsilverio22 - going to give this to you to take a stab at it

cjellick avatar Oct 13 '22 15:10 cjellick

acorn.io/app-namespace=<namespace the App was created in, 'acorn' by default>" acorn.io/app-name=<name of the app, in Mohamed's case 'test-app'>" @cjellick Trying to get this to solve the issue as a starting point should this be added to the Acornfile or to the namespace through something like kubectl label namespaces ns-test-app acorn.io/app-name=test-app?

jsilverio22 avatar Oct 13 '22 23:10 jsilverio22

Yeah, if you are using a namespace pre-created out-of-band from acorn, you should label it explicitly

cjellick avatar Oct 14 '22 14:10 cjellick

@jsilverio22 this also needs documented as a feature. Probably in the "Running acorn apps" section in a new page called "Namespaces and Service Accounts" and you can also document that all containers launched as part of an acorn will have a service account named "acorn".

cjellick avatar Oct 14 '22 14:10 cjellick

Not sure what happened here, but this isnt fixed properly. Here's the cli output:

$ acorn ps
still-leaf   ghcr.io/acorn-io/library/hello-world                          3m3s ago                                                                                                            [controller: not processed]

Though, the appInstance does have a condition in error state with the correct output:

  - error: true
    lastTransitionTime: "2022-11-18T03:58:15Z"
    message: 'can not use namespace abc, existing namespace must have labels acorn.io/app-namespace
      and acorn.io/app-name (Apply Using: kubectl label namespaces abc acorn.io/app-name=still-leaf
      kubectl label namespaces abc acorn.io/app-namespace=acorn) Namespace will be
      deleted when the app is deleted'
    observedGeneration: 1

cjellick avatar Nov 18 '22 04:11 cjellick

Ahh so it’s not actually printing out to the console?

jsilverio22 avatar Nov 18 '22 04:11 jsilverio22

looks good 👍

cjellick avatar Nov 18 '22 16:11 cjellick