You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by "lfabriko (via GitHub)" <gi...@apache.org> on 2024/01/28 18:38:02 UTC
[PR] Compute digest of configmap and secret from its data [camel-k]
lfabriko opened a new pull request, #5115:
URL: https://github.com/apache/camel-k/pull/5115
Proposal of fix to https://github.com/apache/camel-k/issues/5114 - use Name, Namespace and Data/StringData to compute digest of Configmap and Secret.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Compute digest of configmap and secret from its data [camel-k]
Posted by "lfabriko (via GitHub)" <gi...@apache.org>.
lfabriko commented on PR #5115:
URL: https://github.com/apache/camel-k/pull/5115#issuecomment-1938184477
I observe failed tests `TestRunDevMode` (in `common-it-single-operator-installation`) and `TestKamelCLIPromote` (in `common-it-custom-operator-installation`); I tried re-running them on lfabriko github ([attempts 4-8](https://github.com/lfabriko/camel-k/actions/runs/7822740828), branch lfab-ad-5114 rebased on commit https://github.com/apache/camel-k/pull/5144) - `common-it-single-operator-installation` passed in 4, 5, 6, 8; `common-it-custom-operator-installation` passed in 4, 6.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Compute digest of configmap and secret from its data [camel-k]
Posted by "lfabriko (via GitHub)" <gi...@apache.org>.
lfabriko commented on PR #5115:
URL: https://github.com/apache/camel-k/pull/5115#issuecomment-1914277833
I think it tries to compute digest of configmap from memory address: (digest.go)
```
// Configmap and secret content
for _, cm := range configmaps {
//DEBUG
if cm != nil {
log.Infof(fmt.Sprintf("LFAB3 configmap cm.String: %s", cm.String()))
} else {
log.Infof(fmt.Sprintf("LFAB3 cm is nil"))
}
//END OF DEBUG
if _, err := hash.Write([]byte(cm.String())); err != nil {
return "", err
}
}
```
I observed (in build_kit.go) before cm.yaml configmap is loaded, `secrets, configmaps := getIntegrationSecretsAndConfigmaps(ctx, action.client, integration)` returns `[nil]` as `configmaps` (it doesn't change digest).
When the cm is loaded, `digest.ComputeForIntegration` computes hash from whole configmap object via cm.String(), it seems to me it takes also the memory address, which keeps changing, so next step in build_kit.go `if hash != integration.Status.Digest {` is always different.
After adding debugging notes:
```
{"level":"info","ts":"2024-01-29T09:12:00Z","logger":"camel-k","msg":"LFAB3 cm is nil"}
...
{"level":"info","ts":"2024-01-29T09:13:00Z","logger":"camel-k","msg":"LFAB3 configmap cm.String: &ConfigMap{ObjectMeta:{myconfigmap lfabriko 6c693d68-2f0c-46b8-a5b3-a75cb5219211 60808454 0 2024-01-29 09:12:42 +0000 UTC <nil> <nil> map[] map[kubectl.kubernetes.io/last-applied-configuration:{\"apiVersion\":\"v1\",\"data\":{\"bar\":\"2\",\"foo\":\"1\"},\"kind\":\"ConfigMap\",\"metadata\":{\"annotations\":{},\"name\":\"myconfigmap\",\"namespace\":\"lfabriko\",\"ownerReferences\":[{\"apiVersion\":\"camel.apache.org/v1\",\"blockOwnerDeletion\":true,\"controller\":true,\"kind\":\"Integration\",\"name\":\"myintegration\",\"uid\":\"48e97e9e-6eb8-4cb2-9f9b-736521f16ee8\"}]}}\n] [{camel.apache.org/v1 Integration myintegration 48e97e9e-6eb8-4cb2-9f9b-736521f16ee8 0xc0012acd2d 0xc0012acd2e}] [] [{kubectl-client-side-apply Update v1 2024-01-29 09:12:42 +0000 UTC FieldsV1 {\"f:data\":{\".\":{},\"f:bar\":{},\"f:foo\":{}},\"f:metadata\":{\"f:annotations\":{\".\":{},\"f:kubectl.kubernetes.io/last
-applied-configuration\":{}},\"f:ownerReferences\":{\".\":{},\"k:{\\\"uid\\\":\\\"48e97e9e-6eb8-4cb2-9f9b-736521f16ee8\\\"}\":{}}}} }]},Data:map[string]string{bar: 2,foo: 1,},BinaryData:map[string][]byte{},Immutable:nil,}"}
{"level":"info","ts":"2024-01-29T09:13:00Z","logger":"camel-k","msg":"LFAB3 digest vEQhvrHzidhCwTIGgGASlVTKlaW9-pFHuV08xkGVQxsQ"}
{"level":"info","ts":"2024-01-29T09:13:00Z","logger":"camel-k","msg":"LFAB3 configmap cm.String: &ConfigMap{ObjectMeta:{myconfigmap lfabriko 6c693d68-2f0c-46b8-a5b3-a75cb5219211 60808454 0 2024-01-29 09:12:42 +0000 UTC <nil> <nil> map[] map[kubectl.kubernetes.io/last-applied-configuration:{\"apiVersion\":\"v1\",\"data\":{\"bar\":\"2\",\"foo\":\"1\"},\"kind\":\"ConfigMap\",\"metadata\":{\"annotations\":{},\"name\":\"myconfigmap\",\"namespace\":\"lfabriko\",\"ownerReferences\":[{\"apiVersion\":\"camel.apache.org/v1\",\"blockOwnerDeletion\":true,\"controller\":true,\"kind\":\"Integration\",\"name\":\"myintegration\",\"uid\":\"48e97e9e-6eb8-4cb2-9f9b-736521f16ee8\"}]}}\n] [{camel.apache.org/v1 Integration myintegration 48e97e9e-6eb8-4cb2-9f9b-736521f16ee8 0xc0012ace07 0xc0012ace08}] [] [{kubectl-client-side-apply Update v1 2024-01-29 09:12:42 +0000 UTC FieldsV1 {\"f:data\":{\".\":{},\"f:bar\":{},\"f:foo\":{}},\"f:metadata\":{\"f:annotations\":{\".\":{},\"f:kubectl.kubernetes.io/last
-applied-configuration\":{}},\"f:ownerReferences\":{\".\":{},\"k:{\\\"uid\\\":\\\"48e97e9e-6eb8-4cb2-9f9b-736521f16ee8\\\"}\":{}}}} }]},Data:map[string]string{bar: 2,foo: 1,},BinaryData:map[string][]byte{},Immutable:nil,}"}
{"level":"info","ts":"2024-01-29T09:13:00Z","logger":"camel-k","msg":"LFAB3 digest vra0Ihy7JAv6YXdpCCbcno8lWWM7MP6ud38zXLdW-0J0"}
{"level":"info","ts":"2024-01-29T09:13:00Z","logger":"camel-k","msg":"LFAB3 configmap cm.String: &ConfigMap{ObjectMeta:{myconfigmap lfabriko 6c693d68-2f0c-46b8-a5b3-a75cb5219211 60808454 0 2024-01-29 09:12:42 +0000 UTC <nil> <nil> map[] map[kubectl.kubernetes.io/last-applied-configuration:{\"apiVersion\":\"v1\",\"data\":{\"bar\":\"2\",\"foo\":\"1\"},\"kind\":\"ConfigMap\",\"metadata\":{\"annotations\":{},\"name\":\"myconfigmap\",\"namespace\":\"lfabriko\",\"ownerReferences\":[{\"apiVersion\":\"camel.apache.org/v1\",\"blockOwnerDeletion\":true,\"controller\":true,\"kind\":\"Integration\",\"name\":\"myintegration\",\"uid\":\"48e97e9e-6eb8-4cb2-9f9b-736521f16ee8\"}]}}\n] [{camel.apache.org/v1 Integration myintegration 48e97e9e-6eb8-4cb2-9f9b-736521f16ee8 0xc001ca42e0 0xc001ca42e1}] [] [{kubectl-client-side-apply Update v1 2024-01-29 09:12:42 +0000 UTC FieldsV1 {\"f:data\":{\".\":{},\"f:bar\":{},\"f:foo\":{}},\"f:metadata\":{\"f:annotations\":{\".\":{},\"f:kubectl.kubernetes.io/last
-applied-configuration\":{}},\"f:ownerReferences\":{\".\":{},\"k:{\\\"uid\\\":\\\"48e97e9e-6eb8-4cb2-9f9b-736521f16ee8\\\"}\":{}}}} }]},Data:map[string]string{bar: 2,foo: 1,},BinaryData:map[string][]byte{},Immutable:nil,}"}
{"level":"info","ts":"2024-01-29T09:13:00Z","logger":"camel-k","msg":"LFAB3 digest vzFRVYBlBtbp_8dPiB9Zx5A9gLj33R8YGOZV9xHTbCkI"}
```
I tried it against both 5115 and config_reload_test.go.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Compute digest of configmap and secret from its data [camel-k]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #5115:
URL: https://github.com/apache/camel-k/pull/5115#issuecomment-1926572913
:heavy_check_mark: Unit test coverage report - coverage increased from 35.6% to 35.7% (**+0.1%**)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Compute digest of configmap and secret from its data [camel-k]
Posted by "lfabriko (via GitHub)" <gi...@apache.org>.
lfabriko commented on PR #5115:
URL: https://github.com/apache/camel-k/pull/5115#issuecomment-1931709137
I observe failed `TestRunIncrementalBuildWithDifferentBaseImages`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Compute digest of configmap and secret from its data [camel-k]
Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez commented on PR #5115:
URL: https://github.com/apache/camel-k/pull/5115#issuecomment-1931733676
I've restarted the check, maybe it's a flaky one. Please, have a look at the validate error. Locally you can `make lint` to see what's the problem.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Compute digest of configmap and secret from its data [camel-k]
Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez commented on code in PR #5115:
URL: https://github.com/apache/camel-k/pull/5115#discussion_r1473974795
##########
pkg/util/digest/digest.go:
##########
@@ -183,7 +183,7 @@ func ComputeForIntegration(integration *v1.Integration, configmaps []*corev1.Con
//StringData with sorted keys
if s.StringData != nil {
Review Comment:
BinaryData for CM would be required as well.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Compute digest of configmap and secret from its data [camel-k]
Posted by "lfabriko (via GitHub)" <gi...@apache.org>.
lfabriko commented on PR #5115:
URL: https://github.com/apache/camel-k/pull/5115#issuecomment-1933140256
Validation via `make lint` with golangci-lint version 1.55.2 locally passed for modified files.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Compute digest of configmap and secret from its data [camel-k]
Posted by "lfabriko (via GitHub)" <gi...@apache.org>.
lfabriko commented on PR #5115:
URL: https://github.com/apache/camel-k/pull/5115#issuecomment-1914386234
I observe if configmap is created without the ownerReference part, there isn't the memory reference and the case 5115 doesn't appear:
```
{"level":"info","ts":"2024-01-29T09:28:41Z","logger":"camel-k","msg":"LFAB3 configmap cm.String: &ConfigMap{ObjectMeta:{myconfigmap lfabriko fe6aab0c-d21a-4628-b5f6-a2d945c4b9a6 60826770 0 2024-01-29 09:24:23 +0000 UTC <nil> <nil> map[] map[kubectl.kubernetes.io/last-applied-configuration:{\"apiVersion\":\"v1\",\"data\":{\"bar\":\"2\",\"foo\":\"1\"},\"kind\":\"ConfigMap\",\"metadata\":{\"annotations\":{},\"name\":\"myconfigmap\",\"namespace\":\"lfabriko\"}}\n] [] [] [{kubectl-client-side-apply Update v1 2024-01-29 09:24:23 +0000 UTC FieldsV1 {\"f:data\":{\".\":{},\"f:bar\":{},\"f:foo\":{}},\"f:metadata\":{\"f:annotations\":{\".\":{},\"f:kubectl.kubernetes.io/last-applied-configuration\":{}}}} }]},Data:map[string]string{bar: 2,foo: 1,},BinaryData:map[string][]byte{},Immutable:nil,}"}
{"level":"info","ts":"2024-01-29T09:28:41Z","logger":"camel-k","msg":"LFAB3 digest v7q7PJwFnLoEd4_wTiIOwkIRVaZHha7n77tCOXz2KH50"}
{"level":"info","ts":"2024-01-29T09:28:41Z","logger":"camel-k","msg":"LFAB3 configmap cm.String: &ConfigMap{ObjectMeta:{myconfigmap lfabriko fe6aab0c-d21a-4628-b5f6-a2d945c4b9a6 60826770 0 2024-01-29 09:24:23 +0000 UTC <nil> <nil> map[] map[kubectl.kubernetes.io/last-applied-configuration:{\"apiVersion\":\"v1\",\"data\":{\"bar\":\"2\",\"foo\":\"1\"},\"kind\":\"ConfigMap\",\"metadata\":{\"annotations\":{},\"name\":\"myconfigmap\",\"namespace\":\"lfabriko\"}}\n] [] [] [{kubectl-client-side-apply Update v1 2024-01-29 09:24:23 +0000 UTC FieldsV1 {\"f:data\":{\".\":{},\"f:bar\":{},\"f:foo\":{}},\"f:metadata\":{\"f:annotations\":{\".\":{},\"f:kubectl.kubernetes.io/last-applied-configuration\":{}}}} }]},Data:map[string]string{bar: 2,foo: 1,},BinaryData:map[string][]byte{},Immutable:nil,}"}
{"level":"info","ts":"2024-01-29T09:28:41Z","logger":"camel-k","msg":"LFAB3 digest v7q7PJwFnLoEd4_wTiIOwkIRVaZHha7n77tCOXz2KH50"}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Compute digest of configmap and secret from its data [camel-k]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #5115:
URL: https://github.com/apache/camel-k/pull/5115#issuecomment-1929114604
:heavy_check_mark: Unit test coverage report - coverage increased from 35.6% to 35.7% (**+0.1%**)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Compute digest of configmap and secret from its data [camel-k]
Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez commented on code in PR #5115:
URL: https://github.com/apache/camel-k/pull/5115#discussion_r1473899524
##########
pkg/util/digest/digest.go:
##########
@@ -183,7 +183,7 @@ func ComputeForIntegration(integration *v1.Integration, configmaps []*corev1.Con
//StringData with sorted keys
if s.StringData != nil {
Review Comment:
Stringdata should be avoided as Kubernetes will translate all those value in the correspondent base64 in `data`. You can remove this part entirely.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Compute digest of configmap and secret from its data [camel-k]
Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez commented on PR #5115:
URL: https://github.com/apache/camel-k/pull/5115#issuecomment-1918665281
A part the validate check failure, the rest seems good. However, I think it would be convenient to have some unit test to cover the scenarios we're trying to fix.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Compute digest of configmap and secret from its data [camel-k]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #5115:
URL: https://github.com/apache/camel-k/pull/5115#issuecomment-1933500720
:heavy_check_mark: Unit test coverage report - coverage increased from 35.6% to 35.8% (**+0.2%**)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Compute digest of configmap and secret from its data [camel-k]
Posted by "lfabriko (via GitHub)" <gi...@apache.org>.
lfabriko commented on code in PR #5115:
URL: https://github.com/apache/camel-k/pull/5115#discussion_r1473911442
##########
pkg/util/digest/digest.go:
##########
@@ -183,7 +183,7 @@ func ComputeForIntegration(integration *v1.Integration, configmaps []*corev1.Con
//StringData with sorted keys
if s.StringData != nil {
Review Comment:
Thank you for info; also I am not sure if there shouldn't be also `binaryData` included in digest computation in case of `Configmap`...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Compute digest of configmap and secret from its data [camel-k]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #5115:
URL: https://github.com/apache/camel-k/pull/5115#issuecomment-1926487375
:warning: Unit test coverage report - coverage decreased from 35.6% to 35.5% (**-0.1%**)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Compute digest of configmap and secret from its data [camel-k]
Posted by "lfabriko (via GitHub)" <gi...@apache.org>.
lfabriko commented on PR #5115:
URL: https://github.com/apache/camel-k/pull/5115#issuecomment-1926513656
Sure, I thought I should replace it by e2e test.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Compute digest of configmap and secret from its data [camel-k]
Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez commented on PR #5115:
URL: https://github.com/apache/camel-k/pull/5115#issuecomment-1926500293
@lfabriko any reason why you removed the `digest_test.go` in the last commit? I think that was still good to make some unit test coverage. In fact, with that change, the coverage decreased. Can you include them back? Having an e2e does not exclude the presence of unit test.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Compute digest of configmap and secret from its data [camel-k]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #5115:
URL: https://github.com/apache/camel-k/pull/5115#issuecomment-1920672786
:heavy_check_mark: Unit test coverage report - coverage increased from 35.6% to 35.7% (**+0.1%**)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Compute digest of configmap and secret from its data [camel-k]
Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez merged PR #5115:
URL: https://github.com/apache/camel-k/pull/5115
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org