Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[K8S][HELM] Comment license in helm chart NOTES #4313

Closed

Conversation

dnskr
Copy link
Contributor

@dnskr dnskr commented Feb 11, 2023

Why are the changes needed?

The changes are needed to hide license text printed after chart installed.
Before changes:

$ helm install kyuubi ${KYUUBI_HOME}/charts/kyuubi -n kyuubi --create-namespace
NAME: kyuubi
LAST DEPLOYED: Sat Feb 11 20:35:52 2023
NAMESPACE: kyuubi
STATUS: deployed
REVISION: 1
TEST SUITE: None
NOTES:
#
# Licensed to the Apache Software Foundation (ASF) under one or more
# contributor license agreements.  See the NOTICE file distributed with
# this work for additional information regarding copyright ownership.
# The ASF licenses this file to You under the Apache License, Version 2.0
# (the "License"); you may not use this file except in compliance with
# the License.  You may obtain a copy of the License at
#
#    http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
#

The chart has been installed!

In order to check the release status, use:
  helm status kyuubi -n kyuubi
    or for more detailed info
  helm get all kyuubi -n kyuubi

************************
******* Services *******
************************
THRIFT_BINARY:
- To access kyuubi-thrift-binary service within the cluster, use the following URL:
    kyuubi-thrift-binary.kyuubi.svc.cluster.local
- To access kyuubi-thrift-binary service from outside the cluster for debugging, run the following command:
    kubectl port-forward svc/kyuubi-thrift-binary 10009:10009 -n kyuubi
  and use 127.0.0.1:10009

After changes:

$ helm install kyuubi ${KYUUBI_HOME}/charts/kyuubi -n kyuubi --create-namespace
NAME: kyuubi
LAST DEPLOYED: Sat Feb 11 20:37:45 2023
NAMESPACE: kyuubi
STATUS: deployed
REVISION: 1
TEST SUITE: None
NOTES:
The chart has been installed!

In order to check the release status, use:
  helm status kyuubi -n kyuubi
    or for more detailed info
  helm get all kyuubi -n kyuubi

************************
******* Services *******
************************
THRIFT_BINARY:
- To access kyuubi-thrift-binary service within the cluster, use the following URL:
    kyuubi-thrift-binary.kyuubi.svc.cluster.local
- To access kyuubi-thrift-binary service from outside the cluster for debugging, run the following command:
    kubectl port-forward svc/kyuubi-thrift-binary 10009:10009 -n kyuubi
  and use 127.0.0.1:10009

How was this patch tested?

  • Add some test cases that check the changes thoroughly including negative and positive cases if possible

  • Add screenshots for manual tests if appropriate

  • Run test locally before make a pull request

@codecov-commenter
Copy link

Codecov Report

Merging #4313 (7cc7663) into master (de15dbe) will decrease coverage by 0.06%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##             master    #4313      +/-   ##
============================================
- Coverage     53.43%   53.38%   -0.06%     
  Complexity       13       13              
============================================
  Files           560      560              
  Lines         30569    30569              
  Branches       4139     4139              
============================================
- Hits          16334    16318      -16     
- Misses        12702    12715      +13     
- Partials       1533     1536       +3     
Impacted Files Coverage Δ
.../kyuubi/server/mysql/constant/MySQLErrorCode.scala 13.84% <0.00%> (-6.16%) ⬇️
...uubi/engine/spark/events/SparkOperationEvent.scala 88.88% <0.00%> (-5.56%) ⬇️
...ache/kyuubi/server/mysql/MySQLCommandHandler.scala 77.77% <0.00%> (-4.05%) ⬇️
...ache/kyuubi/operation/KyuubiOperationManager.scala 80.00% <0.00%> (-2.67%) ⬇️
...ache/kyuubi/server/mysql/MySQLGenericPackets.scala 76.59% <0.00%> (-2.13%) ⬇️
.../kyuubi/engine/spark/operation/ExecutePython.scala 81.46% <0.00%> (-0.87%) ⬇️
...rc/main/scala/org/apache/spark/ui/EnginePage.scala 78.97% <0.00%> (-0.29%) ⬇️
...apache/kyuubi/engine/JpsApplicationOperation.scala 77.41% <0.00%> (ø)
...kyuubi/engine/KubernetesApplicationOperation.scala 22.53% <0.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@pan3793 pan3793 added this to the v1.7.0 milestone Feb 12, 2023
@pan3793 pan3793 closed this in 7f86611 Feb 12, 2023
pan3793 pushed a commit that referenced this pull request Feb 12, 2023
### _Why are the changes needed?_
The changes are needed to hide license text printed after chart installed.
Before changes:
```
$ helm install kyuubi ${KYUUBI_HOME}/charts/kyuubi -n kyuubi --create-namespace
NAME: kyuubi
LAST DEPLOYED: Sat Feb 11 20:35:52 2023
NAMESPACE: kyuubi
STATUS: deployed
REVISION: 1
TEST SUITE: None
NOTES:
#
# Licensed to the Apache Software Foundation (ASF) under one or more
# contributor license agreements.  See the NOTICE file distributed with
# this work for additional information regarding copyright ownership.
# The ASF licenses this file to You under the Apache License, Version 2.0
# (the "License"); you may not use this file except in compliance with
# the License.  You may obtain a copy of the License at
#
#    http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
#

The chart has been installed!

In order to check the release status, use:
  helm status kyuubi -n kyuubi
    or for more detailed info
  helm get all kyuubi -n kyuubi

************************
******* Services *******
************************
THRIFT_BINARY:
- To access kyuubi-thrift-binary service within the cluster, use the following URL:
    kyuubi-thrift-binary.kyuubi.svc.cluster.local
- To access kyuubi-thrift-binary service from outside the cluster for debugging, run the following command:
    kubectl port-forward svc/kyuubi-thrift-binary 10009:10009 -n kyuubi
  and use 127.0.0.1:10009
```

After changes:
```
$ helm install kyuubi ${KYUUBI_HOME}/charts/kyuubi -n kyuubi --create-namespace
NAME: kyuubi
LAST DEPLOYED: Sat Feb 11 20:37:45 2023
NAMESPACE: kyuubi
STATUS: deployed
REVISION: 1
TEST SUITE: None
NOTES:
The chart has been installed!

In order to check the release status, use:
  helm status kyuubi -n kyuubi
    or for more detailed info
  helm get all kyuubi -n kyuubi

************************
******* Services *******
************************
THRIFT_BINARY:
- To access kyuubi-thrift-binary service within the cluster, use the following URL:
    kyuubi-thrift-binary.kyuubi.svc.cluster.local
- To access kyuubi-thrift-binary service from outside the cluster for debugging, run the following command:
    kubectl port-forward svc/kyuubi-thrift-binary 10009:10009 -n kyuubi
  and use 127.0.0.1:10009
```

### _How was this patch tested?_
- [ ] Add some test cases that check the changes thoroughly including negative and positive cases if possible

- [ ] Add screenshots for manual tests if appropriate

- [ ] [Run test](https://kyuubi.readthedocs.io/en/master/develop_tools/testing.html#running-tests) locally before make a pull request

Closes #4313 from dnskr/comment_license_in_helm_chart_notes.

Closes #4313

7cc7663 [dnskr] [K8S][HELM] Comment license in helm chart NOTES

Authored-by: dnskr <[email protected]>
Signed-off-by: Cheng Pan <[email protected]>
(cherry picked from commit 7f86611)
Signed-off-by: Cheng Pan <[email protected]>
@pan3793
Copy link
Member

pan3793 commented Feb 12, 2023

Thanks, merged to master/1.7

@dnskr dnskr deleted the comment_license_in_helm_chart_notes branch May 11, 2023 20:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants