"[e2e] "imagePullPolicy: Always" and "--metrics-bind-addr=127.0.0.1:8080" replacements not needed anymore?"

invidian commented 2 years ago

/kind bug

What steps did you take and what happened:

While playing with e2e configuration, I noticed that patterns for replacement in upstream manifests may not be applies to used manifests, as upstream manifests no longer include such patterns.

Logs:

$ for i in --metrics-bind-addr=127.0.0.1:8080 --metrics-bind-addr= 'imagePullPolicy:|Always' imagePullPolicy:; do i=$(echo $i | sed 's/|/ /g'); for j in $(git grep -B 10 -- "${i}" | grep https | awk '{print $3}' | sed 's/"//g' | sort -u); do echo "Matches for pattern '${i}' in ${j}"; curl -s -L $j | grep -- "${i}"; done; done
Matches for pattern '--metrics-bind-addr=127.0.0.1:8080' in https://github.com/kubernetes-sigs/cluster-api/releases/download/v0.3.23/core-components.yaml
Matches for pattern '--metrics-bind-addr=127.0.0.1:8080' in https://github.com/kubernetes-sigs/cluster-api/releases/download/v0.4.4/bootstrap-components.yaml
Matches for pattern '--metrics-bind-addr=127.0.0.1:8080' in https://github.com/kubernetes-sigs/cluster-api/releases/download/v0.4.4/control-plane-components.yaml
Matches for pattern '--metrics-bind-addr=127.0.0.1:8080' in https://github.com/kubernetes-sigs/cluster-api/releases/download/v0.4.4/core-components.yaml
Matches for pattern '--metrics-bind-addr=127.0.0.1:8080' in https://github.com/kubernetes-sigs/cluster-api/releases/download/v1.0.2/bootstrap-components.yaml
Matches for pattern '--metrics-bind-addr=127.0.0.1:8080' in https://github.com/kubernetes-sigs/cluster-api/releases/download/v1.0.2/control-plane-components.yaml
Matches for pattern '--metrics-bind-addr=127.0.0.1:8080' in https://github.com/kubernetes-sigs/cluster-api/releases/download/v1.0.2/core-components.yaml
Matches for pattern '--metrics-bind-addr=127.0.0.1:8080' in https://storage.googleapis.com/artifacts.k8s-staging-cluster-api.appspot.com/components/nightly_main_20211122/bootstrap-components.yaml
Matches for pattern '--metrics-bind-addr=127.0.0.1:8080' in https://storage.googleapis.com/artifacts.k8s-staging-cluster-api.appspot.com/components/nightly_main_20211122/control-plane-components.yaml
Matches for pattern '--metrics-bind-addr=127.0.0.1:8080' in https://storage.googleapis.com/artifacts.k8s-staging-cluster-api.appspot.com/components/nightly_main_20211122/core-components.yaml
Matches for pattern '--metrics-bind-addr=' in https://github.com/kubernetes-sigs/cluster-api/releases/download/v0.3.23/core-components.yaml
Matches for pattern '--metrics-bind-addr=' in https://github.com/kubernetes-sigs/cluster-api/releases/download/v0.4.4/bootstrap-components.yaml
        - --metrics-bind-addr=localhost:8080
Matches for pattern '--metrics-bind-addr=' in https://github.com/kubernetes-sigs/cluster-api/releases/download/v0.4.4/control-plane-components.yaml
        - --metrics-bind-addr=localhost:8080
Matches for pattern '--metrics-bind-addr=' in https://github.com/kubernetes-sigs/cluster-api/releases/download/v0.4.4/core-components.yaml
        - --metrics-bind-addr=localhost:8080
Matches for pattern '--metrics-bind-addr=' in https://github.com/kubernetes-sigs/cluster-api/releases/download/v1.0.2/bootstrap-components.yaml
        - --metrics-bind-addr=localhost:8080
Matches for pattern '--metrics-bind-addr=' in https://github.com/kubernetes-sigs/cluster-api/releases/download/v1.0.2/control-plane-components.yaml
        - --metrics-bind-addr=localhost:8080
Matches for pattern '--metrics-bind-addr=' in https://github.com/kubernetes-sigs/cluster-api/releases/download/v1.0.2/core-components.yaml
        - --metrics-bind-addr=localhost:8080
Matches for pattern '--metrics-bind-addr=' in https://github.com/kubernetes-sigs/cluster-api/releases/download/v1.1.0-beta.1/bootstrap-components.yaml
        - --metrics-bind-addr=localhost:8080
Matches for pattern '--metrics-bind-addr=' in https://github.com/kubernetes-sigs/cluster-api/releases/download/v1.1.0-beta.1/control-plane-components.yaml
        - --metrics-bind-addr=localhost:8080
Matches for pattern '--metrics-bind-addr=' in https://github.com/kubernetes-sigs/cluster-api/releases/download/v1.1.0-beta.1/core-components.yaml
        - --metrics-bind-addr=localhost:8080
Matches for pattern '--metrics-bind-addr=' in https://storage.googleapis.com/artifacts.k8s-staging-cluster-api.appspot.com/components/nightly_main_20211122/bootstrap-components.yaml
        - --metrics-bind-addr=localhost:8080
Matches for pattern '--metrics-bind-addr=' in https://storage.googleapis.com/artifacts.k8s-staging-cluster-api.appspot.com/components/nightly_main_20211122/control-plane-components.yaml
        - --metrics-bind-addr=localhost:8080
Matches for pattern '--metrics-bind-addr=' in https://storage.googleapis.com/artifacts.k8s-staging-cluster-api.appspot.com/components/nightly_main_20211122/core-components.yaml
        - --metrics-bind-addr=localhost:8080
Matches for pattern '--metrics-bind-addr=' in https://storage.googleapis.com/artifacts.k8s-staging-cluster-api.appspot.com/components/nightly_main_20220101/bootstrap-components.yaml
        - --metrics-bind-addr=localhost:8080
Matches for pattern '--metrics-bind-addr=' in https://storage.googleapis.com/artifacts.k8s-staging-cluster-api.appspot.com/components/nightly_main_20220101/control-plane-components.yaml
        - --metrics-bind-addr=localhost:8080
Matches for pattern '--metrics-bind-addr=' in https://storage.googleapis.com/artifacts.k8s-staging-cluster-api.appspot.com/components/nightly_main_20220101/core-components.yaml
        - --metrics-bind-addr=localhost:8080
Matches for pattern 'imagePullPolicy: Always' in https://github.com/kubernetes-sigs/cluster-api/releases/download/v0.3.23/core-components.yaml
Matches for pattern 'imagePullPolicy: Always' in https://github.com/kubernetes-sigs/cluster-api/releases/download/v0.4.4/bootstrap-components.yaml
Matches for pattern 'imagePullPolicy: Always' in https://github.com/kubernetes-sigs/cluster-api/releases/download/v0.4.4/control-plane-components.yaml
Matches for pattern 'imagePullPolicy: Always' in https://github.com/kubernetes-sigs/cluster-api/releases/download/v0.4.4/core-components.yaml
Matches for pattern 'imagePullPolicy: Always' in https://github.com/kubernetes-sigs/cluster-api/releases/download/v1.0.2/bootstrap-components.yaml
Matches for pattern 'imagePullPolicy: Always' in https://github.com/kubernetes-sigs/cluster-api/releases/download/v1.0.2/control-plane-components.yaml
Matches for pattern 'imagePullPolicy: Always' in https://github.com/kubernetes-sigs/cluster-api/releases/download/v1.0.2/core-components.yaml
Matches for pattern 'imagePullPolicy: Always' in https://storage.googleapis.com/artifacts.k8s-staging-cluster-api.appspot.com/components/nightly_main_20211122/bootstrap-components.yaml
Matches for pattern 'imagePullPolicy: Always' in https://storage.googleapis.com/artifacts.k8s-staging-cluster-api.appspot.com/components/nightly_main_20211122/control-plane-components.yaml
Matches for pattern 'imagePullPolicy: Always' in https://storage.googleapis.com/artifacts.k8s-staging-cluster-api.appspot.com/components/nightly_main_20211122/core-components.yaml
Matches for pattern 'imagePullPolicy:' in https://github.com/kubernetes-sigs/cluster-api/releases/download/v0.3.23/core-components.yaml
        imagePullPolicy: IfNotPresent
        imagePullPolicy: IfNotPresent
Matches for pattern 'imagePullPolicy:' in https://github.com/kubernetes-sigs/cluster-api/releases/download/v0.4.4/bootstrap-components.yaml
        imagePullPolicy: IfNotPresent
Matches for pattern 'imagePullPolicy:' in https://github.com/kubernetes-sigs/cluster-api/releases/download/v0.4.4/control-plane-components.yaml
        imagePullPolicy: IfNotPresent
Matches for pattern 'imagePullPolicy:' in https://github.com/kubernetes-sigs/cluster-api/releases/download/v0.4.4/core-components.yaml
        imagePullPolicy: IfNotPresent
Matches for pattern 'imagePullPolicy:' in https://github.com/kubernetes-sigs/cluster-api/releases/download/v1.0.2/bootstrap-components.yaml
        imagePullPolicy: IfNotPresent
Matches for pattern 'imagePullPolicy:' in https://github.com/kubernetes-sigs/cluster-api/releases/download/v1.0.2/control-plane-components.yaml
        imagePullPolicy: IfNotPresent
Matches for pattern 'imagePullPolicy:' in https://github.com/kubernetes-sigs/cluster-api/releases/download/v1.0.2/core-components.yaml
        imagePullPolicy: IfNotPresent
Matches for pattern 'imagePullPolicy:' in https://storage.googleapis.com/artifacts.k8s-staging-cluster-api.appspot.com/components/nightly_main_20211122/bootstrap-components.yaml
        imagePullPolicy: IfNotPresent
Matches for pattern 'imagePullPolicy:' in https://storage.googleapis.com/artifacts.k8s-staging-cluster-api.appspot.com/components/nightly_main_20211122/control-plane-components.yaml
        imagePullPolicy: IfNotPresent
Matches for pattern 'imagePullPolicy:' in https://storage.googleapis.com/artifacts.k8s-staging-cluster-api.appspot.com/components/nightly_main_20211122/core-components.yaml
        imagePullPolicy: IfNotPresent

What did you expect to happen:

I'd expect one of 3 things:

If patterns are no longer needed, they should be removed.
If patterns are desired, there should be tests covering their requirement (e.g. in case of --metrics-bind-addr, metrics should be queried as part of e2e test suite to verify they are exposed)
If patterns to are included as a preventive measure, they should be annotated with comment with reasoning, as their effect is not measurable.

Anything else you would like to add:

It seems in revision 80ddb77c, replace statements were indeed doing something:

$ curl -s -L https://github.com/kubernetes-sigs/cluster-api/releases/download/v0.3.10/bootstrap-components.yaml | grep -- --metrics-addr=127.0.0.1:8080
        - --metrics-addr=127.0.0.1:8080
        - --metrics-addr=127.0.0.1:8080

Additionally, perhaps CAPI test framework should generate a warning when replacement is not applied or replacement should have a configurable option to fail when no matches are found. Do you think it's worth creating an issue in CAPI repo for this?

Environment:

Cluster-api-provider-aws version: 67116614

sedefsavas commented 2 years ago

We still have those fields in the manifests published, is this only related to e2e tests?

invidian commented 2 years ago

Yes, this is only for e2e tests, as it is only about CAPI manifests, not CAPA.

k8s-triage-robot commented 2 years ago

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

After 90d of inactivity, lifecycle/stale is applied
After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

Mark this issue or PR as fresh with /remove-lifecycle stale
Mark this issue or PR as rotten with /lifecycle rotten
Close this issue or PR with /close
Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

invidian commented 2 years ago

/remove-lifecycle stale

This seems to me like something which should be addressed by maintainers.

k8s-triage-robot commented 1 year ago

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

After 90d of inactivity, lifecycle/stale is applied
After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

Mark this issue or PR as fresh with /remove-lifecycle stale
Mark this issue or PR as rotten with /lifecycle rotten
Close this issue or PR with /close
Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

invidian commented 1 year ago

It seems it's related to local development #3225?

k8s-triage-robot commented 1 year ago

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

After 90d of inactivity, lifecycle/stale is applied
After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

Mark this issue or PR as fresh with /remove-lifecycle rotten
Close this issue or PR with /close
Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

invidian commented 1 year ago

/remove-lifecycle rotten

k8s-triage-robot commented 1 year ago

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

After 90d of inactivity, lifecycle/stale is applied
After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

Mark this issue or PR as fresh with /remove-lifecycle stale
Mark this issue or PR as rotten with /lifecycle rotten
Close this issue or PR with /close
Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

invidian commented 1 year ago

/remove-lifecycle stale

Ankitasw commented 1 year ago

I think this should have been solved by #3225 , isn't it @invidian ?

invidian commented 1 year ago

I think this should have been solved by https://github.com/kubernetes-sigs/cluster-api-provider-aws/issues/3225 , isn't it @invidian ?

I don't think so. I still see:

          replacements:
            - old: "imagePullPolicy: Always"
              new: "imagePullPolicy: IfNotPresent"
            - old: --metrics-bind-addr=127.0.0.1:8080
              new: --metrics-bind-addr=:8080

For example for source https://github.com/kubernetes-sigs/cluster-api/releases/download/v1.2.6/core-components.yaml in test/e2e/data/e2e_conf.yaml, which does not contain those statements. So as far as I understand, this replacements should be treated appropriately, as mentioned in opening post.

dlipovetsky commented 1 year ago

/area testing /retitle "[e2e] "imagePullPolicy: Always" and "--metrics-bind-addr=127.0.0.1:8080" replacements not needed anymore?" /triage accepted /kind cleanup /priority important-longterm

k8s-triage-robot commented 1 week ago

This issue has not been updated in over 1 year, and should be re-triaged.

You can:

Confirm that this issue is still relevant with /triage accepted (org members only)
Close this issue with /close

For more details on the triage process, see https://www.kubernetes.dev/docs/guide/issue-triage/

/remove-triage accepted

kubernetes-sigs / cluster-api-provider-aws

"[e2e] "imagePullPolicy: Always" and "--metrics-bind-addr=127.0.0.1:8080" replacements not needed anymore?" #3055