fix: issue-5041 #5042
Merged
fix: issue-5041 #5042
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
cndoit18
force-pushed
the
fix-issue-5041
branch
2 times, most recently
from
df83db7 to
a16ce91
Compare
|
That seems to fix it. I've added the full log-output with One thing - I've noticied this Heres a similar run from version 1.8.0 for comparison |
|
I just ran the test again and got the bug again. Don't know if something changed between df83db7 and a16ce91 ? I'll be AFK for an hour or so and then I'll try the test again. For reference, this is the script I use for testing: #!/usr/bin/env bash
set -exuo pipefail
IFS=$'\n\t'
SCRIPT_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
cd "${SCRIPT_DIR}"
# Clone and build the operator sdk
if [[ ! -d operator-sdk ]]; then
git clone [email protected]:operator-framework/operator-sdk.git
fi
cd operator-sdk
# Modify the following to check out the revision you want to test.
# Make sure we're in sync with head of the branch we're trackingt || true
git remote add cndoit18 [email protected]:cndoit18/operator-sdk.git || true
git checkout --track cndoit18/fix-issue-5041 || true
git fetch cndoit18
git reset --hard cndoit18/fix-issue-5041
cd -
# Create the chart, clearing out old versions.
if [[ -d chart ]]; then
rm -fr chart
fi
mkdir -p chart/helmloop/templates
cat <<EOF > chart/helmloop/Chart.yaml
apiVersion: v2
appVersion: 1.0
description: test chart
name: loop
type: application
version: 1.0
EOF
cat <<EOF > chart/helmloop/templates/config.yaml
apiVersion: v1
kind: ConfigMap
metadata:
name: testmap
data:
test-key: "test-value"
EOF
touch chart/helmloop/values.yaml
touch chart/helmloop/NOTES.txt
# Build the operator-sdk cli and the base-image we'll need for the operator.
cd operator-sdk
make build
make image/helm-operator
cd -
# Remove built operator if it exists.
if [[ -d operator ]]; then
rm -fr operator
fi
mkdir operator
cd operator
# Create an operator for the chart
../operator-sdk/build/operator-sdk init --plugins helm --helm-chart ../chart/helmloop/
# Overwrite the dockerfile, in effect just updating the FROM. This allows us
# to inject our previously built base image.
cat <<EOF >./Dockerfile
# Build the manager binary
FROM quay.io/operator-framework/helm-operator:dev
# FROM quay.io/operator-framework/helm-operator:v1.8.0
ENV HOME=/opt/helm
COPY watches.yaml ${HOME}/watches.yaml
COPY helm-charts ${HOME}/helm-charts
WORKDIR ${HOME}
EOF
# Add debugging.
cat <<EOF >>./config/default/kustomization.yaml
patches:
- patch: |-
- op: add
path: /spec/template/spec/containers/1/args/-
value: --zap-devel
- op: add
path: /spec/template/spec/containers/1/imagePullPolicy
value: IfNotPresent
target:
kind: Deployment
EOF
# Build the operator docker image
make docker-build
# Setup the cluster
kind delete cluster --name helmloop || true
kind create cluster --name helmloop
# Load our operator docker image
kind load docker-image --name helmloop controller:latest
# Deploy, wait for it to be ready
make install deploy
kubectl wait --namespace operator-system \
--for=condition=ready pod \
--timeout="60s" \
--selector="control-plane=controller-manager"
# Install a single instance of our crd.
kubectl create namespace loop-install
kubectl apply -n loop-install -f config/samples/charts_*
# Monitor the managers logs for looping.
kubectl -n operator-system logs deployment/operator-controller-manager -c manager -f |
|
@danquah Thank you very much, I will verify it against your script |
cndoit18
force-pushed
the
fix-issue-5041
branch
5 times, most recently
from
5e0971f to
7dc744f
Compare
|
I have verified it, please try again @danquah |
achton
reviewed
Jul 6, 2021
|
I'll give it a whirl - brb |
cndoit18
force-pushed
the
fix-issue-5041
branch
2 times, most recently
from
c8f2f22 to
caced7a
Compare
|
New test-run of commit c8f2f22 - seems to work - I'll do a new run of caced7a and report back ... |
openshift-ci
bot
added
the
do-not-merge/work-in-progress
Please wait a moment, I'll verify the other cases |
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
|
@danquah It has solved. 👋 |
|
Awesome! - I'll run a test right away and report back. |
cndoit18
force-pushed
the
fix-issue-5041
branch
2 times, most recently
from
710da63 to
ca0e907
Compare
|
I will use this script to check #4937 |
|
You are most welcome and thank you for your hard work 🙏🏾 |
|
/cherry-pick v1.8.x |
|
/cherry-pick v1.9.x |
|
@estroz: new pull request created: #5045 In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
@estroz: new pull request created: #5046 In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
@cndoit18 Sorry I didn't see this and comment sooner. It looks like we're now using the templating inputs in our comparisons (i.e. the chart and the config) rather than the output (the resulting manifest). I'm curious if you considered cases where changes to the templating execution itself could cause a template to be rendered differently such that comparing just the chart and config would be insufficient? For example, perhaps a helm release pulls in a new version of the sprig template library that fixes a bug in the way a template works. Or perhaps the helm-operator introduces a post-renderer that injects some custom labels or annotations necessary for a new helm-operator feature to work. In either of these cases, a user may upgrade their operator to pull these changes in, but then not actually receive a Helm release upgrade until they make a change somewhere in their chart or custom object's spec. |
It is possible, I will read the helm code carefully and come up with a better solution. thank you |
cndoit18
added a commit
to cndoit18/operator-sdk
that referenced
this pull request
Jul 14, 2021
Signed-off-by: cndoit18 <[email protected]>
cndoit18
added a commit
to cndoit18/operator-sdk
that referenced
this pull request
Jul 14, 2021
Signed-off-by: cndoit18 <[email protected]>
cndoit18
added a commit
to cndoit18/operator-sdk
that referenced
this pull request
Jul 14, 2021
Signed-off-by: cndoit18 <[email protected]>
cndoit18
added a commit
to cndoit18/operator-sdk
that referenced
this pull request
Jul 14, 2021
Signed-off-by: cndoit18 <[email protected]>
cndoit18
added a commit
to cndoit18/operator-sdk
that referenced
this pull request
Jul 14, 2021
Signed-off-by: cndoit18 <[email protected]>
cndoit18
added a commit
to cndoit18/operator-sdk
that referenced
this pull request
Jul 14, 2021
Signed-off-by: cndoit18 <[email protected]>
cndoit18
added a commit
to cndoit18/operator-sdk
that referenced
this pull request
Jul 14, 2021
Signed-off-by: cndoit18 <[email protected]>
cndoit18
added a commit
to cndoit18/operator-sdk
that referenced
this pull request
Jul 15, 2021
Signed-off-by: cndoit18 <[email protected]>
2 tasks
varshaprasad96
added a commit
to varshaprasad96/operator-sdk
that referenced
this pull request
Jul 28, 2021
This PR reverts: 1. Revert release equality comparison logic ref: operator-framework#5042 2. Changes made to the updated logic which caused charts to upgrade constantly - ref: operator-framework#4937 Signed-off-by: varshaprasad96 <[email protected]>
2 tasks
varshaprasad96
added a commit
to varshaprasad96/operator-sdk
that referenced
this pull request
Aug 2, 2021
This reverts commit 57ac0fe. Signed-off-by: varshaprasad96 <[email protected]>
varshaprasad96
added a commit
that referenced
this pull request
Aug 3, 2021
* Revert "fix: issue-5041 (#5042)" This reverts commit 57ac0fe. Signed-off-by: varshaprasad96 <[email protected]> * Revert "fix(helm): properly compare existing and candidate releases (#4937)" This reverts commit cae12a2. Signed-off-by: varshaprasad96 <[email protected]> * add changelog fragment Signed-off-by: varshaprasad96 <[email protected]>
openshift-cherrypick-robot
pushed a commit
to openshift-cherrypick-robot/operator-sdk
that referenced
this pull request
Aug 3, 2021
This reverts commit 57ac0fe. Signed-off-by: varshaprasad96 <[email protected]>
openshift-cherrypick-robot
pushed a commit
to openshift-cherrypick-robot/operator-sdk
that referenced
this pull request
Aug 3, 2021
This reverts commit 57ac0fe. Signed-off-by: varshaprasad96 <[email protected]>
openshift-cherrypick-robot
pushed a commit
to openshift-cherrypick-robot/operator-sdk
that referenced
this pull request
Aug 3, 2021
This reverts commit 57ac0fe. Signed-off-by: varshaprasad96 <[email protected]>
bentito
pushed a commit
to bentito/operator-sdk
that referenced
this pull request
Aug 24, 2021
…ramework#5097) * Revert "fix: issue-5041 (operator-framework#5042)" This reverts commit 57ac0fe. Signed-off-by: varshaprasad96 <[email protected]> * Revert "fix(helm): properly compare existing and candidate releases (operator-framework#4937)" This reverts commit cae12a2. Signed-off-by: varshaprasad96 <[email protected]> * add changelog fragment Signed-off-by: varshaprasad96 <[email protected]>
twasyl
pushed a commit
to twasyl/operator-sdk
that referenced
this pull request
Sep 3, 2021
…ramework#5097) * Revert "fix: issue-5041 (operator-framework#5042)" This reverts commit 57ac0fe. Signed-off-by: varshaprasad96 <[email protected]> * Revert "fix(helm): properly compare existing and candidate releases (operator-framework#4937)" This reverts commit cae12a2. Signed-off-by: varshaprasad96 <[email protected]> * add changelog fragment Signed-off-by: varshaprasad96 <[email protected]> Signed-off-by: Thierry Wasylczenko <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Signed-off-by: cndoit18 [email protected]
Description of the change:
fix #5041
Motivation for the change:
Checklist
If the pull request includes user-facing changes, extra documentation is required:
changelog/fragments(seechangelog/fragments/00-template.yaml)website/content/en/docs